[00:04] <anastasiamac> thumper: re chairs... it makes sense now why u r advocating standing tables :D
[00:04] <anastasiamac> desks even
[00:13] <anastasiamac> menn0: tyvm for looking :)
[00:13] <anastasiamac> menn0: it's still WIP but I have added a li'l detail to description :)
[00:32] <davecheney> thumper: http://paste.ubuntu.com/11765246/
[00:32] <davecheney> down to three races
[00:32] <davecheney> one is trivial
[00:32] <davecheney> the other two are more complicated to fix
[00:32] <davecheney> actually, its probaly 4
[00:32] <davecheney> 2 are simple
[00:33] <davecheney> nope, the last one is the gocheck issue
[00:33] <davecheney> so, 1 simple, 3 hard
[00:33] <davecheney> urgh
[00:33] <davecheney> i can't cannot
[00:33] <davecheney> count
[00:33] <davecheney> short versoin, there are less races today
[00:33] <davecheney> some are easy to fix
[00:33] <davecheney> some are hard
[00:33] <davecheney> there are less than yesterday
[00:36] <mwhudson> and more than tomorrow?
[00:38] <menn0> anastasiamac: thanks. i've decided to be more picky about that when doing reviews
[00:39] <anastasiamac> menn0: \o/
[01:28] <mup> Bug #1468153 opened: Charms with storage don't use cloud-native default if size is specified, but provider is omitted <storage> <juju-core:Triaged> <https://launchpad.net/bugs/1468153>
[01:31] <thumper> davecheney: I guess that's good
[01:34] <mup> Bug #1468153 changed: Charms with storage don't use cloud-native default if size is specified, but provider is omitted <storage> <juju-core:Triaged> <https://launchpad.net/bugs/1468153>
[01:40] <mup> Bug #1464356 opened: TestCloudInit fails <ci> <gccgo> <regression> <test-failure> <windows> <juju-core:In Progress by thumper> <juju-core 1.24:Fix Committed by thumper> <https://launchpad.net/bugs/1464356>
[01:40] <mup> Bug #1468153 opened: Charms with storage don't use cloud-native default if size is specified, but provider is omitted <storage> <juju-core:Triaged> <https://launchpad.net/bugs/1468153>
[01:45] <axw> wallyworld: updated http://reviews.vapour.ws/r/1994/, PTAL
[01:45] <wallyworld> sure
[01:50] <wallyworld> axw: just to check, as well as  TestRemoveLastVolumeAttachment where we check Dead, there's also a TestRemoveNotLastVolumeAttachment where we assert that the volume is not cleaned up?
[01:51] <axw> wallyworld: we can't do shared storage yet, so no
[01:51] <wallyworld> ok
[01:52] <wallyworld> lgtm
[01:52] <axw> thanks
[01:54] <thumper> axw: seen bug 1466167 ?
[01:54] <mup> Bug #1466167: debug-hooks no longer works with 1.24-beta6 <debug-hooks> <regression> <juju-core:Triaged> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1466167>
[01:54] <thumper> axw: I was thinking that this was fixed now
[01:55] <axw> thumper: hrm, I tested debug-hooks after my latest change to jujuc
[01:55]  * axw tests again
[01:55] <axw> thumper: oh this was fixed when waigani reverted an older change
[01:55] <thumper> axw: I have had reports that it broke again...
[01:55] <axw> status just needs to be updated. will do that
[01:55]  * thumper shrugs
[01:55] <axw> I'll test again anyway
[01:55] <thumper> thanks
[01:56] <thumper> 1.24 and master if you can
[01:56] <thumper> then just update the bug
[01:56] <axw> sure
[01:56] <thumper> I thought you had fixed it
[01:56] <thumper> but then someone said it was broken "again"
[01:56] <thumper> and I wasn't sure how soon again was
[01:56] <thumper> axw: thanks for double checking thought, I appreciate it
[01:57] <axw> thumper: no worries
[01:57] <thumper> hazaar !! \o/
[01:57] <thumper> my ci blocker fix landed in 1.,24 and just now, master
[01:57]  * thumper crosses fingers for a blessing
[01:58] <thumper> menn0: I'll update both jes-cli and db-log with master now that the ci bugs *should* be fixed
[01:59] <thumper> hmm...
[01:59]  * thumper needs caffeine
[01:59] <menn0> thumper: awesome
[02:09] <axw> thumper: all good, updated the bug
[02:09] <thumper> axw: awesome, thanks
[02:15]  * thumper goes to turn on the coffee machine
[02:47] <menn0> thumper: thanks for the reviews
[02:47] <thumper> np
[02:48]  * thumper is looking at what appears to be an intermittent failure
[02:49] <thumper> WT actual F?
[02:50]  * thumper sighs
[02:50] <thumper> labix.org/v2/mgo/socket.go:285 is the only place this error text exists
[02:50] <thumper> which is in the Close() method
[02:51] <thumper> so... how did this test fail with that? http://juju-ci.vapour.ws:8080/job/github-merge-juju/3768/console
[02:51] <thumper> passes here 40 times in a row
[02:51] <thumper> what would close it?
[02:52] <thumper> I don't see anything that would close it...
[02:52] <thumper> bah humbug
[02:53] <natefinch> thumper: I've seen that before, though not recently... I forget now what was causing it.
[02:55] <thumper> natefinch: hmm...
[02:56] <natefinch> thumper: google says gustavo said this about it at one point: "if a machine is explicitly removed
[02:56] <natefinch> from a replica set, mgo will proactively close any open sockets to
[02:56] <natefinch> prevent errors from happening due to logic that stays running with a
[02:56] <natefinch> server that isn't updating."
[02:57] <natefinch> from the same thread, the guy who was hitting this problem: "when this was happening, we were hitting our file descriptor limit"
[02:57] <thumper> this use case doesn't touch any of that AFICT
[02:57] <thumper> huh
[02:57] <thumper> interesting
[03:00] <natefinch> It sounds like there's no one specific reason that it can happen
[03:02] <axw> wallyworld: there was just one bug that affects 1.24, I've put up a PR. the other issues are only to do with destroying storage, so just on master
[03:02] <wallyworld> ty, looking
[03:04] <wallyworld> axw: lgtm
[03:05] <axw> wallyworld: cheers
[03:11] <mup> Bug #1467973 changed: uploadSuite.TearDownTest Fails <ci> <intermittent-failure> <unit-tests> <juju-core:Invalid> <juju-core 1.24:Invalid> <https://launchpad.net/bugs/1467973>
[03:15] <natefinch> good old sudo wget | sh
[03:16] <natefinch> er | sudo sh  whatever you know what I mean
[03:33] <thumper> meh
[03:33]  * thumper jumps through the hoops to get a win2012 server kvm env to test windows test
[03:43] <thumper> damn gready install, wants at least 12.5 gig disk
[03:44] <mup> Bug #1466011 changed: apiserver tests fail on windows <ci> <regression> <test-failure> <windows> <juju-core:Fix Released by dave-cheney> <https://launchpad.net/bugs/1466011>
[03:44] <mup> Bug #1468166 opened: serviceManagerSuite.TestCreate <blocker> <ci> <regression> <test-failure> <juju-core:Incomplete> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1468166>
[04:02] <davecheney> thumper: do you have a second, well more like 10 mins, for a discussion on a race fix ?
[04:03] <thumper> yeah...
[04:04] <thumper> davecheney: hangout choice?
[04:04] <davecheney> 1:1 ?
[04:04] <davecheney> thumper: http://paste.ubuntu.com/11765877/
[04:27] <thumper> davecheney: http://wiki.cloudbase.it/juju-testing
[04:28] <davecheney> thumper: r u doing the standing desk ?
[04:29] <mup> Bug #1438951 changed: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Expired> <https://launchpad.net/bugs/1438951>
[04:30] <thumper> davecheney: yeah
[04:30] <davecheney> i moved my 'desk' down to the lounge room table now I'm on my own for two weeks
[04:30] <davecheney> does that count ?
[04:33] <thumper> well... are you standing up?
[04:35] <davecheney> i have to stand up to go downstairs
[04:35] <davecheney> this is undeniable
[04:44] <mup> Bug #1438951 opened: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Expired> <https://launchpad.net/bugs/1438951>
[04:51] <davecheney> thumper: this race is really worrying
[04:51] <davecheney> i think i need to bug it
[04:52] <thumper> oh... kay...
[04:52] <davecheney> http://paste.ubuntu.com/11766040/
[04:52] <davecheney> this code alters the config value
[04:53] <davecheney> which is really worrying
[04:53] <davecheney> because the watcher fires to indicate st.EnviroConfig() has been updated
[04:53] <davecheney> but that is then passed to environ.New()
[04:53] <davecheney> which _alters_ the config object
[04:53] <davecheney> even if thee was a copy returned from st.EnviornConfig(), which there is not
[04:53] <davecheney> this is still bonkers
[04:56] <mup> Bug #1438951 changed: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Triaged> <https://launchpad.net/bugs/1438951>
[04:58]  * davecheney assumes the foetal position
[04:59] <mup> Bug #1438951 opened: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Triaged> <https://launchpad.net/bugs/1438951>
[05:03] <davecheney> AAAAAAAAAAAAAAAARGH
[05:03] <davecheney> http://paste.ubuntu.com/11766069/
[05:03] <davecheney> watching for the environ config _and_ trying to retrieve the _current_ config at the same time will DEADLOCK
[05:06] <thumper> wallyworld, axw, davecheney: anyone... http://reviews.vapour.ws/r/2015/
[05:07]  * davecheney looking
[05:07] <davecheney> LGTM
[05:07] <thumper> cheers
[05:08] <davecheney> that was pretty simple
[05:08] <thumper> this only compiles and runs on windows
[05:08] <thumper> davecheney: yeah
[05:08] <thumper> most of the effort was in setting up the machine to test in
[05:08] <davecheney> the log would say something like, check failed func(xx32842342) not comparable
[05:11] <thumper>     c.Assert(s.getPasswd.Calls, gc.HasLen, 1)
[05:11] <thumper> ... obtained func() []testing.StubCall = (func() []testing.StubCall)(0x43f760)
[05:11] <thumper> ... n int = 1
[05:11] <thumper> ... obtained value type has no length
[05:11] <thumper> anyway, master doesn't have this problem
[05:11] <thumper> just 1.24
[05:12] <davecheney> yeah, what you said
[05:16] <thumper> sinzui: fix submitted
[05:16]  * thumper goes to start dinner
[05:17] <mup> Bug #1468166 changed: serviceManagerSuite.TestCreate <blocker> <ci> <regression> <test-failure> <juju-core:Invalid> <juju-core 1.24:In Progress by thumper> <https://launchpad.net/bugs/1468166>
[05:47] <mup> Bug #1468187 opened: Juju on Fedora <juju-core:New> <https://launchpad.net/bugs/1468187>
[05:47] <mup> Bug #1468188 opened: environs/config: Validate mutates the config passed to it <juju-core:New> <https://launchpad.net/bugs/1468188>
[08:23] <mup> Bug #1468223 opened: failed units fail to fail and fail to die <juju-core:New> <https://launchpad.net/bugs/1468223>
[09:01] <dimitern> voidspace, TheMue, jam, fwereade, standup?
[09:02] <jam> dimitern: brt, got logged out of google
[13:06] <mup> Bug #1468349 opened: discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily <test-failure> <unit-tests> <wily> <juju-core:Triaged> <https://launchpad.net/bugs/1468349>
[13:15] <mup> Bug #1468349 changed: discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily <test-failure> <unit-tests> <wily> <juju-core:Triaged> <https://launchpad.net/bugs/1468349>
[13:27] <mup> Bug #1468349 opened: discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily <test-failure> <unit-tests> <wily> <juju-core:Triaged> <https://launchpad.net/bugs/1468349>
[13:36] <mup> Bug # opened: 1468354, 1468355, 1468357, 1468359
[13:48] <mup> Bug #1468365 opened: internal compiler error: fault <ci> <intermittent-failure> <juju-core:Incomplete> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1468365>
[14:00] <mup> Bug #1468365 changed: internal compiler error: fault <ci> <intermittent-failure> <juju-core:Incomplete> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1468365>
[14:06] <mup> Bug #1468365 opened: internal compiler error: fault <ci> <intermittent-failure> <juju-core:Incomplete> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1468365>
[14:06] <mup> Bug #1468369 opened: TestBootstrapNoToolsDevelopmentConfig fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1468369>
[14:09] <mup> Bug #1468369 changed: TestBootstrapNoToolsDevelopmentConfig fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1468369>
[14:18] <mup> Bug #1468369 opened: TestBootstrapNoToolsDevelopmentConfig fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1468369>
[14:55] <natefinch> ericsnow: this is what we get back from docker inspect... thoughts on what we should include in our status string?  http://pastebin.ubuntu.com/11768085/
[14:55] <ericsnow> Id
[14:55] <ericsnow> natefinch: wish it was shorter
[14:55] <natefinch> ericsnow: yes but, status
[14:55] <ericsnow> natefinch: oh, right
[14:56] <ericsnow> natefinch: something derived from State
[14:56] <natefinch> ericsnow: and this is where I want to be able to return structured data as state ;)
[14:56] <natefinch> er status
[14:57] <ericsnow> natefinch: I'm not sure that exposing that whole thing to users when they call juju status is the right thing to do
[14:57] <natefinch> ericsnow: not the whole thing, but like one of the statuses (running, etc) and maybe the PID and error if not empty
[14:58] <ericsnow> natefinch: wwitzel3 et al. would probably have better insight into that though
[14:58] <ericsnow> natefinch: you may have a point (PID would be nice in status)
[14:59] <ericsnow> natefinch: let's get some feedback from wwitzel3, whit, etc.
[15:03] <mup> Bug #1464356 changed: TestCloudInit fails <ci> <gccgo> <regression> <test-failure> <windows> <juju-core:Fix Released by thumper> <juju-core 1.24:Fix Released by thumper> <https://launchpad.net/bugs/1464356>
[15:05] <natefinch> ericsnow: I think the "Name" can be used instead of the UUID...
[15:06] <natefinch> ericsnow: in this case it's /dreamy_ptolemy
[15:06] <ericsnow> natefinch: as long as it's unique we don't care :)
[15:06] <natefinch> ericsnow: just might be more user friendly
[15:06] <ericsnow> natefinch: my preference is certainly toward user-friendly
[15:06] <natefinch> ericsnow: and I'm pretty sure it is unique per host machine
[15:06] <ericsnow> natefinch: sounds good
[15:15] <mup> Bug #1464356 opened: TestCloudInit fails <ci> <gccgo> <regression> <test-failure> <windows> <juju-core:Fix Released by thumper> <juju-core 1.24:Fix Released by thumper> <https://launchpad.net/bugs/1464356>
[15:24] <mup> Bug #1464356 changed: TestCloudInit fails <ci> <gccgo> <regression> <test-failure> <windows> <juju-core:Fix Released by thumper> <juju-core 1.24:Fix Released by thumper> <https://launchpad.net/bugs/1464356>
[16:03] <katco> ericsnow: http://reviews.vapour.ws/r/2020/
[16:03] <ericsnow> katco: will review
[16:06] <natefinch> aww man, there's some benefits to docker being a Go project, on the help for "docker inspect": --format=""    Format the output using the given go template.
[16:07] <natefinch> actually, not sure how useful that is in practice, but it *seems*  cool
[16:19] <natefinch> ericsnow: charm.Process has a name, should I be setting the docker name to that?  Is it guaranteed to be unique?  I notice that validate requires it to be non-empty... docker will generate its own name for the container if you don't give it one
[16:21] <ericsnow> natefinch: What happens if you try to launch more than one docker with the same name?
[16:21] <natefinch> ericsnow: it'll probably fail.  The name is supposed to be a unique identifier.  I can try and see what happens
[16:22] <ericsnow> natefinch: could the plugin append some unique suffix to the proc name?
[16:23] <ericsnow> natefinch: actually, for now we could just use the proc name
[16:23] <natefinch> ericsnow: sure.... or maybe we could just allow name to be empty and let the plugin give it a name
[16:23] <ericsnow> natefinch: let's not go there for now :)
[16:24] <natefinch> ericsnow: so for right now we just use whatever we're given and if there's a collision, we return an error and say "don't do that"?
[16:24] <ericsnow> natefinch: exactly
[16:24] <natefinch> ericsnow: k
[16:24] <natefinch> ericsnow: what's "Type"?
[16:25] <ericsnow> natefinch: the plugin name
[16:25] <ericsnow> natefinch: e.g. "docker"
[16:25] <natefinch> ericsnow: oh, ok, so the plugin itself can probably ignore that ;)
[16:25] <ericsnow> natefinch: (a plugin could support more than one type)
[16:25] <ericsnow> natefinch: sure
[16:26] <ericsnow> natefinch: I suppose the plugin could fail if there's an unexpected type, but that really shouldn't be a factor
[16:26] <ericsnow> natefinch: (in Juju anyway)
[16:27] <natefinch> ericsnow: TypeOptions is just arguments to be given to the executable?
[16:27] <ericsnow> natefinch: pretty much
[16:27] <natefinch> ericsnow: I'm not really sure how to translate a map to a list of strings... maybe it would be better if that were just a slice of strings?
[16:28] <natefinch> ericsnow: for example, if you wanted to pass just "-d" or something
[16:28] <ericsnow> natefinch: "--<opt>=<val>"?
[16:29] <ericsnow> natefinch: I expect you'll have to loop over the map and use a switch inside to compose the arg list
[16:29] <natefinch> ericsnow: is there some reason we can't just make it a list of arguments?  That seems like the most direct way to get arguments into the command line
[16:31] <natefinch> ericsnow: I don't really want to have to parse a map and try to guess what people mean.
[16:32] <ericsnow> natefinch: I'm not sure here
[16:32] <ericsnow> natefinch: I see what you mean, but I'm trying to remember why we went with a map
[16:33] <katco> ericsnow: natefinch: haven't followed this conversation too closely, but would the flag package be helpful here?
[16:34] <ericsnow> katco: unfortuantely no
[16:34] <natefinch> katco: not really... this is like the opposite of that
[16:34] <natefinch> katco: I need the unflag package :)
[16:34] <katco> natefinch: ah, taking a list of args and passing them to the command line as flags?
[16:35] <ericsnow> katco: yep
[16:36] <katco> ericsnow: natefinch: i may have something, gimme a few mins
[16:36] <natefinch> katco: it would work fine as []string, but ericsnow is trying to remember why they made it a map
[16:36] <natefinch> katco: I actually think a []string is much more intuitive and less error prone
[16:38] <natefinch> I man, that's what the CLI expects, what exec.Command expects, etc
[16:40] <katco> ericsnow: natefinch: http://play.golang.org/p/CDP_7MagHH
[16:40] <ericsnow> katco: using the flag package is way overkill here
[16:41] <ericsnow> katco: the data is originating in the charm's metadata.yaml
[16:41] <ericsnow> katco: (or similarly formatted at the CLI)
[16:42] <katco> ericsnow: hm. can you elaborate on why? it seems to do what you need in a few lines of code and is robust
[16:42] <natefinch> ericsnow: maybe it's a map so you can override things?
[16:42] <ericsnow> natefinch: that works just as well with a list of strings
[16:43] <ericsnow> natefinch: I think we used a map for reasons that no longer apply
[16:44] <natefinch> katco: it's specified as a map in the yaml, and needs to be a []string for exec.Command
[16:44] <natefinch> katco: except that it doesn't really need to be a map in the yaml
[16:44] <ericsnow> katco: YAML -> flags -> JSON (YAML) -> string vs. YAML -> []string -> YAML -> string
[16:45] <katco> natefinch: ah, i misunderstood. https://github.com/juju/juju/blob/feature-proc-mgmt/process/env.go#L36-L38 ?
[16:46] <natefinch> katco: https://github.com/juju/charm/blob/v5/process.go#L23
[16:46] <ericsnow> natefinch: however, with a map you avoid the ambiguity of duplicates and you lose any expectation of ordering
[16:46] <natefinch> ericsnow: uh... ordering can matter and duplicates can be valid
[16:46] <natefinch> ericsnow: that's just reasons not to use a map :)
[16:46] <ericsnow> natefinch: there are several dedicated fields in charm.Process which are translated into args by the plugin
[16:47] <natefinch> ericsnow: yep
[16:47] <ericsnow> natefinch: I would not expect ordering to matter, but if it does then yeah, a map won't work
[16:48] <ericsnow> natefinch: and yeah duplicates should be respected
[16:48] <ericsnow> natefinch: so I'm pretty much convinced :)
[16:49] <ericsnow> natefinch: how do we resolve the dedicated fields with order?  put them first?
[16:49] <natefinch> ericsnow: *shrug* charms will need to know how to use their respective plugin
[16:49] <natefinch> ericsnow: hopefully order won't matter
[16:50] <ericsnow> natefinch: the plugin has to decide where to stick those values
[16:51] <natefinch> ericsnow: yes... generally the only time order matters is at the very end, like grepping multiple files, the files all go at the end
[16:51] <natefinch> ericsnow: unless we just let them specify the entire arg list, there's no much we can do about ordering
[16:52] <ericsnow> natefinch: at least in the case of those dedicated fields
[16:52] <natefinch> ericsnow: right
[16:52] <natefinch> ericsnow: FWIW, the docker plugin won't have a problem with ordering... only the command is order dependent, so it's easy enough to put that in the right spot
[16:52] <ericsnow> natefinch: we could, as a rule, always put them at the front of the option list (unless they are args)
[16:53] <ericsnow> natefinch: with args they already have a specific order
[16:53] <natefinch> ericsnow: right, so once we switch TypeOptions to a []string, I think we're done
[16:53] <ericsnow> natefinch: k
[16:53] <natefinch> (and call we call it Args, since that's what it is?)
[16:53] <ericsnow> natefinch: please leave it TypeOptions
[16:54] <natefinch> ericsnow: as a charm author, TypeOptions does not sounds like "arguments to pass to the thing you're running"
[16:54] <ericsnow> natefinch: it's the options for the process type
[16:55] <rogpeppe> anyone else seen this error? (local provider, juju-core tip or close to it):
[16:55] <rogpeppe> %  juju deploy -e local wordpress
[16:55] <rogpeppe> ERROR cannot retrieve charm "cs:trusty/wordpress-2": cannot get archive: Get https://api.jujucharms.com/charmstore/v4/trusty/wordpress-2/archive: dial tcp: lookup api.jujucharms.com on [::1]:53: read udp [::1]:35163->[::1]:53: read: connection refused
[16:55] <rogpeppe> i can get the URL just fine from my regular command line
[16:55] <katco> ericsnow: natefinch: http://reviews.vapour.ws/r/2022/
[16:56] <ericsnow> katco: dang it
[16:56] <natefinch> ericsnow: I often find that the doc comment is revealing when looking at naming things.  The doc comment says "TypeOptions is a map of arguments for the process type"
[16:57] <ericsnow> natefinch: that's what I just said :)
[16:57] <natefinch> katco: use jc.SameContents
[16:57] <ericsnow> katco, natefinch: yes, please :)
[16:57] <natefinch> katco: it does slice comparison that ignores order
[16:57] <katco> natefinch: cool, changing it
[16:58] <natefinch> ericsnow: if there's a better word for describing the thing in the comments, that's probably the word you should use for the name.  // Args is a map of arguments for the process type
[16:59] <natefinch> (except, now it's a list, of course)
[16:59] <natefinch> ericsnow: I think the fact that I had to ask you what it was is kind of telling.  If it was called "arguments" or "args" I don't think I would have had to ask.
[16:59] <ericsnow> natefinch: TypeArgs vs. TypeOptions?  Not much of a difference there
[17:00] <katco> ericsnow: natefinch: http://reviews.vapour.ws/r/2022/
[17:00] <ericsnow> natefinch: just Args (or Options) is too vague
[17:01] <natefinch> ericsnow: why type?  the type is Docker or SystemD or Rocket.  The args are for whatever CLI command you're running....  but I'm not sure how to name that
[17:01] <natefinch> ClientArgs?
[17:02] <ericsnow> natefinch: they are options for use by the type
[17:02] <ericsnow> natefinch: the plugin is responsible for interpreting their meaning and translating that into the CLI args
[17:03] <natefinch> ericsnow: ok... let's roll back.  It sounds like these might not always be a direct mapping to CLI args. That might be my misunderstand
[17:04] <natefinch> ing
[17:04] <natefinch> ericsnow: so, like, this could just be "other stuff you need to specify but that we can't forsee for every plugin, so here's map, use that"
[17:05] <ericsnow> natefinch: yep
[17:05] <natefinch> ericsnow: ok, sorry, that was my misunderstanding
[17:06] <natefinch> ericsnow: so this requires some shared knowledge between plugin and charm on what the values in the map mean... and that's fine.
[17:06] <ericsnow> natefinch: the options should still map pretty closely to the args the plugin will use
[17:07] <natefinch> ericsnow: ideally, yes, so that it's easier to understand how to set the options.
[17:09] <ericsnow> natefinch: yep
[17:09] <natefinch> ericsnow: I think it might just call it Options, then.  It's options for the process, the fact that it applies to the process type is implicit.  And I don't think I'd use "arguments" in the comment, because that makes it sound like CLI arguments, and they're almost certainly not a 1:1 mapping (if they were, you could just use a []string and specify CLI arguments directly)
[17:09] <ericsnow> natefinch: we call it TypeOptions because it does not contain any other options (e.g. things used by Juju)
[17:10] <ericsnow> natefinch: "explicit is better than implicit"
[17:10] <katco> err... on build bot: "fatal: unable to access 'https://gopkg.in/mgo.v2/': Could not resolve host: gopkg.in"
[17:11] <natefinch> ericsnow: but they're options for this specific process. Another process of the same type might have different options.  So they're options for this process.  Yes, they're type-dependent, so but so is the command and image etc
[17:12] <ericsnow> natefinch: yeah, but other parts of charm.Process are not type-dependent
[17:12] <natefinch> katco: http://downforeveryoneorjustme.com/gopkg.in
[17:12] <ericsnow> natefinch: and they are specific to that process because that charm.Process{} is specific to that processs :)
[17:12] <katco> natefinch: gustavo has an official uptime link: http://stats.pingdom.com/r29i3cfl66c0
[17:13] <natefinch> ericsnow: the *process* itself is type dependent
[17:13] <ericsnow> natefinch: exactly
[17:13] <katco> natefinch: looks like it was a blip on our end
[17:13] <natefinch> ericsnow: so my point is, type is redundant. The whole struct is type dependent.
[17:14] <ericsnow> natefinch: not the whole struct
[17:15] <natefinch> ericsnow: it is. You're passing it to a specific plugin for that type.  The way "port" is interpreted is type-dependent
[17:15] <natefinch> ericsnow: can we at least change "arguments" to "options" in the comment?
[17:16] <ericsnow> natefinch: sure
[17:16] <natefinch> ericsnow: in my mind arguments == CLI and that is not necessarily the case
[17:16] <ericsnow> natefinch: to me, in this context, they mean exactly the same thing
[17:16] <ericsnow> natefinch: but I see what you mean
[17:16] <natefinch> ericsnow: but that's because you wrote it and you know what you mean.  To someone else, they have to figure out what you mean
[17:16] <ericsnow> natefinch: :)
[17:17] <katco> natefinch: hmm... arguments get passed to functions as well
[17:18] <natefinch> katco: but in this case, this is all stuff that corresponds to how the plugin will launch a process, which is almost certainly via a CLI (I suppose it could be an API of some sort... but I'm guessing it'll usually be a CLI)
[17:22] <natefinch> ericsnow: It seems like it could be "PluginOptions"  .  "type" to me, is an ID.  You can't have options for an ID.  The plugin and the process both have a type, and that's "docker".  But the options are for the Plugin.  Docker itself won't know how to translate that map.
[17:23] <ericsnow> natefinch: the fact that we are using plugins is an implementation detail
[17:23] <ericsnow> natefinch: you could just as well call it TypeHandlerOptions
[17:23] <katco> ericsnow: natefinch let's not bike shed this too much longer plx
[17:23] <natefinch> katco: I wouldn't if it weren't for the fact that it's not changeable later
[17:23] <natefinch> katco: this defines the API between Juju and the plugin
[17:24] <natefinch> and the charm
[17:24] <katco> natefinch: until this feature is live, can't it be changed
[17:24] <katco> ?
[17:24] <natefinch> katco: yes, true
[17:25] <natefinch> ericsnow: how about this... leave the field name and just change the argument to "TypeOptions is a map of options for the process type handler."?
[17:26] <natefinch> s/argument/comment
[17:27] <ericsnow> natefinch: sounds good
[17:27] <natefinch> ericsnow: awesome.   Periwinkle blue it is.
[17:28] <ericsnow> natefinch: lol
[17:28] <ericsnow> natefinch: hey, I thought you said Bullwinkle!
[17:28] <natefinch> Too late!
[17:28] <katco> ericsnow: i'm about ready to pair up once you review that branch
[17:29] <ericsnow> katco: k
[17:29] <katco> natefinch: feel free as well: http://reviews.vapour.ws/r/2020/
[17:31] <natefinch> katco, ericsnow: what's up with the panics, anyway?
[17:31] <ericsnow> natefinch: who's panicking?
[17:32] <natefinch> ericsnow: there's some // TODO(ericsnow)  return an error instead
[17:32] <katco> natefinch: just following the pattern; i just don't think we've defined the error path instead
[17:32] <natefinch> ericsnow: in registerHookContextCommands
[17:32] <katco> natefinch: it's not intended to stay that way obviously
[17:32] <natefinch> katco: obv, just wondering what's up
[17:32] <katco> natefinch: incremental progress, that's what's up ;)
[17:33] <natefinch> oh... is this that stuff that gets called from Init() so panics aren't the end of the world?
[17:34] <katco> natefinch: correct
[17:34] <natefinch> ok.. I get it
[17:34] <katco> natefinch: this is the very tip top of the chain.
[17:34] <katco> natefinch: but my larger point is: this is the way it is because it doesn't matter right now. we can define a better way later
[17:35] <katco> brb tea
[17:59] <katco> ericsnow: moonstone?
[18:16] <natefinch> ericsnow, katco: FYI docker's launch and status pretty much working with all options etc.  Destroy will be trivial too.  Still need tests... gotta run for a little bit, will be on later this afternoon and then more tonight.
[18:16] <ericsnow> natefinch-afk: sweet!
[19:22] <sinzui> katco Do you have a minute to review http://reviews.vapour.ws/r/2023/
[19:22] <katco> sinzui: tal
[19:26] <katco> sinzui: sorry, not sure if you saw: ship it!
[19:27] <sinzui> katco: I did :)
[19:27] <katco> sinzui: k :p
[20:25] <natefinch> ericsnow: should destroy remove the volumes associated with the container?
[20:34] <katco> natefinch: i don't think so
[20:35] <natefinch> katco: k
[20:36] <katco> natefinch: http://reviews.vapour.ws/r/2018/diff/# read the description
[20:40] <natefinch> katco: kk
[20:47] <natefinch> ericsnow: what's command for?
[20:47] <natefinch> ericsnow: (on charm.Process) .... is that like the docker command to run (i.e. docker run <container> <command>?
[20:47] <ericsnow> natefinch: the command to pass to docker (etc.)
[20:47] <ericsnow> natefinch: yep
[20:50] <natefinch> ericsnow: hmm... that probably needs to be a []string then...   for example, if you pass "sleep 30" as the command, then docker tries to run "sleep 30" as the command name
[20:50] <natefinch> (and says there's no command called "sleep 30")
[20:51] <ericsnow> natefinch: then perhaps the plugin should address splitting the string for the user
[20:57] <natefinch> ericsnow: that's incredibly tricky  what if command is 'cowsay "howdy there!"` ?  what quoting scheme do we use, bash, or someting else?
[20:58] <natefinch> ericsnow: there's a reason exec.Command requires the user to split up the args :)
[20:58]  * thumper shakes his fist at shitty tests
[20:59] <natefinch> ericsnow: seems trivial to have the user write Command: [ "cowsay", "howdy there!"]   rather than Command: "cowsay 'howdy there!'"
[20:59] <natefinch> ericsnow: and a lot less error prone
[21:00] <ericsnow> natefinch: what do docker commands normally look like?
[21:00] <natefinch> ericsnow: docker run container cowsay "howdy there!"
[21:11] <natefinch> ericsnow: if Command is just a []string, then I can just do args = append(args, process.Command...) at the end... otherwise I have to get fancy and tricky and assume I know how the user expects their command string to get split up
[21:15] <natefinch> katco, ericsnow: I gotta make dinner, but I'll be back in a little less than 4 hours.... I can make progress even if we don't figure out the command stuff... plenty of tests to write of the rest of the code.
[21:16] <ericsnow> natefinch-afk: yeah, let's discuss the command stuff tomorrow
[21:28] <davecheney> PASS: pinger_test.go:131: mongoPingerSuite.TestAgentConnectionsShutDownWhenStateDies    30.481s
[21:29] <davecheney> thumper
[21:29] <davecheney> exactly 30 seconds to run this test
[21:29]  * davecheney sharpens knife
[21:36] <davecheney> thumper: sorry, can you resend
[21:36] <davecheney> basically nothing is working today
[21:37] <ericsnow> axw: in case you didn't notice, I fixed the PR links in RB (for new PRs) :)
[21:38] <davecheney> ericsnow: thanks
[21:39] <davecheney> that was super annoying
[21:39] <ericsnow> davecheney: agreed
[21:39] <ericsnow> davecheney: sorry it took so long to get a round tuit
[21:39] <davecheney> s'ok
[21:39] <davecheney> fixed now
[21:52] <katco> thumper: for serious, i would love to see coupling of commits/branches <--> bugs
[22:51] <axw> ericsnow: I did, thanks very much
[23:06] <thumper> davecheney: http://reviews.vapour.ws/r/2024/diff/#
[23:15] <thumper> OMG running tests in the win2012 virtual server is SSLLLOOOOWWWW.....
[23:34] <perrito666> thumper: really? It didnt seem that slower