| anastasiamac | thumper: re chairs... it makes sense now why u r advocating standing tables :D | 00:04 |
|---|---|---|
| anastasiamac | desks even | 00:04 |
| anastasiamac | menn0: tyvm for looking :) | 00:13 |
| anastasiamac | menn0: it's still WIP but I have added a li'l detail to description :) | 00:13 |
| 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:32 |
| 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:33 |
| mwhudson | and more than tomorrow? | 00:36 |
| menn0 | anastasiamac: thanks. i've decided to be more picky about that when doing reviews | 00:38 |
| anastasiamac | menn0: \o/ | 00:39 |
| 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:28 |
| thumper | davecheney: I guess that's good | 01:31 |
| 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:34 |
| 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:40 |
| axw | wallyworld: updated http://reviews.vapour.ws/r/1994/, PTAL | 01:45 |
| wallyworld | sure | 01:45 |
| 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:50 |
| axw | wallyworld: we can't do shared storage yet, so no | 01:51 |
| wallyworld | ok | 01:51 |
| wallyworld | lgtm | 01:52 |
| axw | thanks | 01:52 |
| 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:54 |
| 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:55 |
| 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:56 |
| 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:57 | |
| thumper | menn0: I'll update both jes-cli and db-log with master now that the ci bugs *should* be fixed | 01:58 |
| thumper | hmm... | 01:59 |
| * thumper needs caffeine | 01:59 | |
| menn0 | thumper: awesome | 01:59 |
| axw | thumper: all good, updated the bug | 02:09 |
| thumper | axw: awesome, thanks | 02:09 |
| * thumper goes to turn on the coffee machine | 02:15 | |
| menn0 | thumper: thanks for the reviews | 02:47 |
| thumper | np | 02:47 |
| * thumper is looking at what appears to be an intermittent failure | 02:48 | |
| thumper | WT actual F? | 02:49 |
| * 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:50 |
| 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:51 |
| thumper | I don't see anything that would close it... | 02:52 |
| thumper | bah humbug | 02:52 |
| natefinch | thumper: I've seen that before, though not recently... I forget now what was causing it. | 02:53 |
| thumper | natefinch: hmm... | 02:55 |
| === kadams54 is now known as kadams54-away | ||
| 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:56 |
| 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 | 02:57 |
| natefinch | It sounds like there's no one specific reason that it can happen | 03:00 |
| 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:02 |
| wallyworld | axw: lgtm | 03:04 |
| axw | wallyworld: cheers | 03:05 |
| 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:11 |
| natefinch | good old sudo wget | sh | 03:15 |
| natefinch | er | sudo sh whatever you know what I mean | 03:16 |
| thumper | meh | 03:33 |
| * thumper jumps through the hoops to get a win2012 server kvm env to test windows test | 03:33 | |
| thumper | damn gready install, wants at least 12.5 gig disk | 03:43 |
| 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> | 03:44 |
| davecheney | thumper: do you have a second, well more like 10 mins, for a discussion on a race fix ? | 04:02 |
| thumper | yeah... | 04:03 |
| thumper | davecheney: hangout choice? | 04:04 |
| davecheney | 1:1 ? | 04:04 |
| davecheney | thumper: http://paste.ubuntu.com/11765877/ | 04:04 |
| thumper | davecheney: http://wiki.cloudbase.it/juju-testing | 04:27 |
| davecheney | thumper: r u doing the standing desk ? | 04:28 |
| mup | Bug #1438951 changed: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Expired> <https://launchpad.net/bugs/1438951> | 04:29 |
| 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:30 |
| thumper | well... are you standing up? | 04:33 |
| davecheney | i have to stand up to go downstairs | 04:35 |
| davecheney | this is undeniable | 04:35 |
| mup | Bug #1438951 opened: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Expired> <https://launchpad.net/bugs/1438951> | 04:44 |
| davecheney | thumper: this race is really worrying | 04:51 |
| davecheney | i think i need to bug it | 04:51 |
| thumper | oh... kay... | 04:52 |
| davecheney | http://paste.ubuntu.com/11766040/ | 04:52 |
| davecheney | this code alters the config value | 04:52 |
| 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:53 |
| mup | Bug #1438951 changed: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Triaged> <https://launchpad.net/bugs/1438951> | 04:56 |
| * davecheney assumes the foetal position | 04:58 | |
| mup | Bug #1438951 opened: destroy-enviroment --force destroy all aws instances <destroy-environment> <ec2-provider> <juju-core:Triaged> <https://launchpad.net/bugs/1438951> | 04:59 |
| 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:03 |
| thumper | wallyworld, axw, davecheney: anyone... http://reviews.vapour.ws/r/2015/ | 05:06 |
| * davecheney looking | 05:07 | |
| davecheney | LGTM | 05:07 |
| thumper | cheers | 05:07 |
| 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:08 |
| 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:11 |
| davecheney | yeah, what you said | 05:12 |
| thumper | sinzui: fix submitted | 05:16 |
| * thumper goes to start dinner | 05:16 | |
| 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:17 |
| === brandon is now known as Guest50566 | ||
| === Guest50566 is now known as web | ||
| 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> | 05:47 |
| mup | Bug #1468223 opened: failed units fail to fail and fail to die <juju-core:New> <https://launchpad.net/bugs/1468223> | 08:23 |
| dimitern | voidspace, TheMue, jam, fwereade, standup? | 09:01 |
| jam | dimitern: brt, got logged out of google | 09:02 |
| mup | Bug #1468349 opened: discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily <test-failure> <unit-tests> <wily> <juju-core:Triaged> <https://launchpad.net/bugs/1468349> | 13:06 |
| mup | Bug #1468349 changed: discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily <test-failure> <unit-tests> <wily> <juju-core:Triaged> <https://launchpad.net/bugs/1468349> | 13:15 |
| mup | Bug #1468349 opened: discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily <test-failure> <unit-tests> <wily> <juju-core:Triaged> <https://launchpad.net/bugs/1468349> | 13:27 |
| mup | Bug # opened: 1468354, 1468355, 1468357, 1468359 | 13:36 |
| mup | Bug #1468365 opened: internal compiler error: fault <ci> <intermittent-failure> <juju-core:Incomplete> <juju-core 1.24:Triaged> <https://launchpad.net/bugs/1468365> | 13:48 |
| 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:00 |
| 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:06 |
| mup | Bug #1468369 changed: TestBootstrapNoToolsDevelopmentConfig fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1468369> | 14:09 |
| mup | Bug #1468369 opened: TestBootstrapNoToolsDevelopmentConfig fails <ci> <intermittent-failure> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1468369> | 14:18 |
| 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:55 |
| 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:56 |
| 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:57 |
| 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:58 |
| ericsnow | natefinch: let's get some feedback from wwitzel3, whit, etc. | 14:59 |
| 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:03 |
| natefinch | ericsnow: I think the "Name" can be used instead of the UUID... | 15:05 |
| 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:06 |
| 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:15 |
| 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:24 |
| katco | ericsnow: http://reviews.vapour.ws/r/2020/ | 16:03 |
| ericsnow | katco: will review | 16:03 |
| 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:06 |
| natefinch | actually, not sure how useful that is in practice, but it *seems* cool | 16:07 |
| 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:19 |
| 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:21 |
| ericsnow | natefinch: could the plugin append some unique suffix to the proc name? | 16:22 |
| 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:23 |
| 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:24 |
| 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:25 |
| 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:26 |
| 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:27 |
| natefinch | ericsnow: for example, if you wanted to pass just "-d" or something | 16:28 |
| ericsnow | natefinch: "--<opt>=<val>"? | 16:28 |
| 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:29 |
| natefinch | ericsnow: I don't really want to have to parse a map and try to guess what people mean. | 16:31 |
| 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:32 |
| katco | ericsnow: natefinch: haven't followed this conversation too closely, but would the flag package be helpful here? | 16:33 |
| 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:34 |
| ericsnow | katco: yep | 16:35 |
| 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:36 |
| natefinch | I man, that's what the CLI expects, what exec.Command expects, etc | 16:38 |
| katco | ericsnow: natefinch: http://play.golang.org/p/CDP_7MagHH | 16:40 |
| ericsnow | katco: using the flag package is way overkill here | 16:40 |
| ericsnow | katco: the data is originating in the charm's metadata.yaml | 16:41 |
| ericsnow | katco: (or similarly formatted at the CLI) | 16:41 |
| 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:42 |
| ericsnow | natefinch: I think we used a map for reasons that no longer apply | 16:43 |
| 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:44 |
| katco | natefinch: ah, i misunderstood. https://github.com/juju/juju/blob/feature-proc-mgmt/process/env.go#L36-L38 ? | 16:45 |
| 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:46 |
| 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:47 |
| ericsnow | natefinch: and yeah duplicates should be respected | 16:48 |
| ericsnow | natefinch: so I'm pretty much convinced :) | 16:48 |
| 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:49 |
| ericsnow | natefinch: the plugin has to decide where to stick those values | 16:50 |
| 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:51 |
| 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:52 |
| 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:53 |
| 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:54 |
| 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:55 |
| 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:56 |
| 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:57 |
| 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:58 |
| 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 | 16:59 |
| katco | ericsnow: natefinch: http://reviews.vapour.ws/r/2022/ | 17:00 |
| ericsnow | natefinch: just Args (or Options) is too vague | 17:00 |
| 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:01 |
| 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:02 |
| 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:03 |
| 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:04 |
| ericsnow | natefinch: yep | 17:05 |
| natefinch | ericsnow: ok, sorry, that was my misunderstanding | 17:05 |
| 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:06 |
| natefinch | ericsnow: ideally, yes, so that it's easier to understand how to set the options. | 17:07 |
| 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:09 |
| 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:10 |
| 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:11 |
| 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:12 |
| 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:13 |
| ericsnow | natefinch: not the whole struct | 17:14 |
| 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:15 |
| 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:16 |
| katco | natefinch: hmm... arguments get passed to functions as well | 17:17 |
| 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:18 |
| 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:22 |
| 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:23 |
| 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:24 |
| 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:25 |
| natefinch | s/argument/comment | 17:26 |
| ericsnow | natefinch: sounds good | 17:27 |
| natefinch | ericsnow: awesome. Periwinkle blue it is. | 17:27 |
| 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:28 |
| ericsnow | katco: k | 17:29 |
| katco | natefinch: feel free as well: http://reviews.vapour.ws/r/2020/ | 17:29 |
| natefinch | katco, ericsnow: what's up with the panics, anyway? | 17:31 |
| ericsnow | natefinch: who's panicking? | 17:31 |
| 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:32 |
| natefinch | oh... is this that stuff that gets called from Init() so panics aren't the end of the world? | 17:33 |
| 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:34 |
| katco | brb tea | 17:35 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| katco | ericsnow: moonstone? | 17:59 |
| === kadams54 is now known as kadams54-away | ||
| 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 |
| === natefinch is now known as natefinch-afk | ||
| ericsnow | natefinch-afk: sweet! | 18:16 |
| === kadams54-away is now known as kadams54 | ||
| === kadams54 is now known as kadams54-away | ||
| sinzui | katco Do you have a minute to review http://reviews.vapour.ws/r/2023/ | 19:22 |
| katco | sinzui: tal | 19:22 |
| katco | sinzui: sorry, not sure if you saw: ship it! | 19:26 |
| sinzui | katco: I did :) | 19:27 |
| katco | sinzui: k :p | 19:27 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| === natefinch-afk is now known as natefinch | ||
| natefinch | ericsnow: should destroy remove the volumes associated with the container? | 20:25 |
| katco | natefinch: i don't think so | 20:34 |
| natefinch | katco: k | 20:35 |
| katco | natefinch: http://reviews.vapour.ws/r/2018/diff/# read the description | 20:36 |
| natefinch | katco: kk | 20:40 |
| === kadams54 is now known as kadams54-away | ||
| 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:47 |
| 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:50 |
| ericsnow | natefinch: then perhaps the plugin should address splitting the string for the user | 20:51 |
| natefinch | ericsnow: that's incredibly tricky what if command is 'cowsay "howdy there!"` ? what quoting scheme do we use, bash, or someting else? | 20:57 |
| 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:58 | |
| 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 | 20:59 |
| ericsnow | natefinch: what do docker commands normally look like? | 21:00 |
| natefinch | ericsnow: docker run container cowsay "howdy there!" | 21:00 |
| 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:11 |
| 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:15 |
| === natefinch is now known as natefinch-afk | ||
| ericsnow | natefinch-afk: yeah, let's discuss the command stuff tomorrow | 21:16 |
| davecheney | PASS: pinger_test.go:131: mongoPingerSuite.TestAgentConnectionsShutDownWhenStateDies 30.481s | 21:28 |
| davecheney | thumper | 21:29 |
| davecheney | exactly 30 seconds to run this test | 21:29 |
| * davecheney sharpens knife | 21:29 | |
| davecheney | thumper: sorry, can you resend | 21:36 |
| davecheney | basically nothing is working today | 21:36 |
| ericsnow | axw: in case you didn't notice, I fixed the PR links in RB (for new PRs) :) | 21:37 |
| davecheney | ericsnow: thanks | 21:38 |
| 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:39 |
| katco | thumper: for serious, i would love to see coupling of commits/branches <--> bugs | 21:52 |
| axw | ericsnow: I did, thanks very much | 22:51 |
| thumper | davecheney: http://reviews.vapour.ws/r/2024/diff/# | 23:06 |
| thumper | OMG running tests in the win2012 virtual server is SSLLLOOOOWWWW..... | 23:15 |
| perrito666 | thumper: really? It didnt seem that slower | 23:34 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!