[07:38] fwereade_: morning! [08:52] Morning [08:58] TheMue: hiyta [08:58] hiya [08:59] rogpeppe: I'm now returning a more detailed error containing the stderr output. [08:59] TheMue: cool [09:00] rogpeppe: Could you take a look again? [09:00] TheMue: yeah, in a little bit [09:00] rogpeppe: Thx. === TheMue_ is now known as TheMue [10:33] * rogpeppe loves it when he starts implementing something, then realises it's unexpectedly done already, as a natural consequence of earlier changes. [10:33] davecheney, dimitern: hiya [10:35] rehowdy [10:38] rogpeppe: hey [10:39] davecheney, dimitern: Morning. [10:41] rogpeppe: Any chance to take a look? [10:41] TheMue: sorry, i'm on a roll this morning - i don't want to derail for the moment, sorry [10:42] rogpeppe: OK [10:42] TheMue: i'll take a look after i've dealt with this branch [10:42] rogpeppe: Would be great, thank you. [10:52] TheMue: morning [10:58] TheMue: you've got a review [10:58] rogpeppe: Thx [11:01] wallyworld: mgz and me are on mumble [11:02] rogpeppe: Read it and the first two points are relative simple, ok. But the multi-line stderr is indeed a problem. I don't know if it should be part of the one-liner error string. For those who wnat to use it it's in the error as a field. [11:02] TheMue: i know. there's no easy answer i'm afraid, but i'm not sure it's right as is. [11:03] TheMue: (and i've seen lxc print multi-line error messages, so i know it definitely is an issue) [11:04] rogpeppe: My problem also is, that the scope of that CL moved. The intention has been to add "first tests" to extend it later. ;) [11:04] TheMue: i think the error handling is rightly part of the first tests [12:03] fwereade_: ping [12:06] rogpeppe: Aaargh, found an interesting behavior. The error message of lxc is written to stdout, not stderr. Will change the code according, os/exec supports it. [12:06] TheMue: ha. [12:07] TheMue: it depends on the command maybe [12:07] TheMue: the command i tried printed it to stderr [12:07] TheMue: which command did you try? [12:08] rogpeppe: Yes, maybe. Strange. But I should get it with the combined output. [12:08] rogpeppe: I took lxc-create [12:09] TheMue: the problem is that some commands might print an error *and* some expected standard output [12:09] TheMue: lxc-start prints to stderr... [12:09] TheMue: yuck [12:10] TheMue: and lxc-create doesn't prefix its error lines either [12:10] rogpeppe: I'll find my way. So far I already test most stuff before I even start a command and I'll add a root check too. ;) [12:10] rogpeppe: Yes, that's what Aram said, it's inconsistent. [12:10] TheMue: yeah, it's badly done [12:11] TheMue: perhaps we should file a bug report [12:11] rogpeppe: I'll make a note, yes. [12:12] TheMue: i think there might be a pattern. i suspect that the lxc commands that are shell scripts print errors to stdout; the ones that aren't print errors to stderr [12:13] TheMue: and their errors are totally inconsistent too. check out this one: [12:13] echo "E: lxc-info - no such file" >&2 [12:15] rogpeppe: I just take all output for debugging, additionally to the returned ExecError of exec.Command [12:15] TheMue: yeah, i don't think there's much else you can do actually [12:16] TheMue: even some of the commands that are expected to produce stuff to stdout still print their errors to stdout. [12:16] TheMue: what a friggin' mess. i don't think people know how to use shell scripts any more. [12:17] rogpeppe: Hehe, yep. [12:33] TheMue: https://sourceforge.net/tracker/?func=detail&aid=3589389&group_id=163076&atid=826303 [12:33] rogpeppe: Great [12:34] rogpeppe: Thx, I would have done it later. ;) [12:34] TheMue: it annoyed me enough that i wanted to do it there and then :-) [12:34] rogpeppe: You're using it @home? [12:35] TheMue: no, just the principle of it :-) [12:35] rogpeppe: Hehe :D [12:51] lunchtime, brb [13:01] niemeyer: yo! [13:02] niemeyer: i'm just passing the ca-cert to jujud, and i'm not quite sure of the best way to do it. i could use a flag or a file. [13:02] rogpeppe: Heya [13:02] niemeyer: i've just done it as a flag, but i'm not sure that's right. [13:02] rogpeppe: I'm not quite sure either.. it should be a file [13:03] rogpeppe: But we could pass as a flag, or hardcode the path [13:03] niemeyer: currently my best thought is to hard-code the path (inside datadir) [13:04] niemeyer: the flag is awkward because i'm not sure how the upstart config copes with multi-line strings [13:05] niemeyer: ah, you mean pass the filename as a flag [13:05] niemeyer: that could be better actually (and marginally easier to test, possibly) [13:06] niemeyer: i think i'll go with that for the time being. [13:07] rogpeppe: Sounds good [13:12] niemeyer: this branch might finally be ok now, i'm hoping: https://codereview.appspot.com/6850087/ [13:13] niemeyer: and i'm hoping this one too: https://codereview.appspot.com/6855054/ [13:15] rogpeppe: Cool, I'll be there son [13:15] soon [13:16] niemeyer: "son" works too, in a slightly patronising kinda way :-) [13:47] rogpeppe: So, changes are in. [13:47] TheMue: ok [14:03] TheMue: reviewed [14:03] rogpeppe: Great, thank you. [14:16] rogpeppe: Closer, review sent [14:16] niemeyer: quick crack test: is what i'm doing in TestPackage here ok, or crackful? the alternative is to add something in every SetUpSuite. https://codereview.appspot.com/6842088/diff/1/cmd/jujud/main_test.go [14:16] niemeyer: thanks [14:18] "What if the path is != "" and m[attr] is nil, shouldn't it stay nil?" [14:18] niemeyer: i don't *think* so [14:18] niemeyer: i deliberately chose that behaviour [14:18] niemeyer: to follow the documentation [14:20] niemeyer: in particular "The ...-path key is translated into "..." by loading the content from the respective file" [14:21] niemeyer: i thought that meant that it's best to honour the path value if it's set [14:22] niemeyer: note that providing a nil value still prevents it from "reading the standard paths" [14:31] rogpeppe: I'm not sure I see what you're trying to achieve [14:32] rogpeppe: I thought you had suggested yesterday that it shouldn't change if it's nil? [14:32] niemeyer: i'm trying to come up with something that's not too surprising to someone that reads the docs on New() [14:32] niemeyer: path always overrides value [14:32] niemeyer: (currently) [14:32] niemeyer: if you've explicitly specified a path, i think it would be surprising if it was ignored [14:33] rogpeppe: Okay [14:41] I'll get a quick lunch [14:43] dimitern: can I bother you for a review of the swift fixup stuff? [14:43] * rogpeppe should probably get lunch too [14:44] niemeyer: PTAL https://codereview.appspot.com/6850087 [14:56] dimitern: i'm seeing an openstack test failure in trunk [14:56] dimitern: http://paste.ubuntu.com/1379598/ [14:56] * dimitern looks [14:57] rogpeppe: hmm.. strange, why didn't it fail before when I run the test.. [14:57] dimitern: i know not [14:58] rogpeppe: it's a trivial fix, I'm on it [14:58] dimitern: it might be possible that you're relying on map-traversal order [14:58] dimitern: (which is random) [14:59] rogpeppe: maybe, anyway it was the last change [14:59] reminds me, I should actually get the juju-core suite passing here [14:59] it was unhappy last time I ran it [15:02] mgz: when you have a test failure, pastebin it, and i'll let you know if it's a common one or not [15:02] mgz: there are still a few sporadic test failures [15:02] mgz: mostly to do with unreliability of external components, i think [15:03] rogpeppe: strange, I cannot see the openstack provider code at all in trunk [15:03] dimitern: that's odd [15:04] rogpeppe: it should be there, right? it was yesterday [15:04] dimitern: i see it [15:04] dimitern: what do the last few entries of the revision log look like? [15:05] rogpeppe: well, problem #0 is what command to use to run the tests, and problem #1 is what I think I should use runs some, but then has: [15:05] go build launchpad.net/juju-core/cmd/juju: signal 9 [15:05] FAIL launchpad.net/juju-core/cmd/juju [build failed] [15:05] which seems non-passy [15:05] rogpeppe: well, mostly yours - up to rev 734 [15:05] mgz: to run all the tests, go to the juju-core root dir, and type "go test ./..." [15:06] okay, so that is the convention. any ideas on the build fail? [15:06] rogpeppe: and doing either bzr up or bzr pull does not get me anything new [15:07] dimitern: trunk is up to revision 737 [15:07] dimitern: what does bzr info print? [15:07] dimitern: are you in the right branch, and what's the parent?... what rogpeppe said [15:08] * rogpeppe really goes for some lunch now [15:08] mgz: I'm at the master branch (as cobzr names trunk) [15:09] dimitern@kubrik:~/work/juju-core$ bzr up [15:09] Tree is up to date at revision 734 of branch /home/dimitern/work/go/src/launchpad.net/juju-core/.bzr/cobzr/master [15:09] dimitern@kubrik:~/work/juju-core$ bzr pull [15:09] Using saved parent location: /home/dimitern/work/go/src/launchpad.net/juju-core/ [15:09] No revisions or tags to pull. [15:09] okay, so that's the problem [15:09] mgz: what? [15:10] the parent branch is a local mirror, not lp:juju [15:10] *lp:juju-core [15:10] mgz: I see, so how to change that? [15:10] use `bzr pull --remember lp:juju-core` [15:10] mgz: nice, 10x [15:19] mgz, rogpeppe: PTAL https://codereview.appspot.com/6843109 [15:25] dimitern: I still get failures on those tests, but from a different issue [15:26] mgz: can I see the paste? [15:26] I don't get any errors, and I run the tests like 30 times in a row [15:26] it's likely because this box is set up differently to yours, and the tests are not well isolated from home [15:27] so, specifically there are no keys in ~/.ssh [15:27] mgz: so it's just happening on yours [15:27] and your test stuff doesn't pass authorized-keys or similar [15:28] no, it's not at all to do with ssh [15:28] the failures I'm seeing :) [15:28] mgz: got you :) so not in the OS provider tests [15:29] yes, those tests, but different failures from the one you're fixing [15:29] rogpeppe: LGTM [15:30] niemeyer: YAY! [15:30] mgz: can I still see the paste? [15:31] when pastebin starts responding... [15:31] http://pastebin.ubuntu.com/1379685/ [15:31] :) [15:32] mgz: I see, but that's from environs/config or something beneath [15:33] rogpeppe, re the cert work, how does the client know the endpoint / cert fingerprint is valid for the endpoint? [15:33] hazmat: the client has a CA public cert [15:33] full run with some failures from outside the openstack dir: [15:33] http://pastebin.ubuntu.com/1379695/ [15:33] hazmat: i'm not sure how that works from a web-browser perspective [15:34] guess I'll fix up some of these [15:34] mgz: so put an ssh pubkey then :) [15:34] rogpeppe, k, but where is the private cert for the CA? [15:34] dimitern: if the test is loading stuff from your homedir to work, it's not a very good test [15:35] tests should pass on a clean box, not just are carefully set up development environment [15:36] mgz: i agree, sure, so which test is actually the root cause? [15:36] mgz: until recently there have been a few tests like that. i've fixed them, hopefully in some of my latest branches [15:36] rogpeppe: any that still need reviewing/landing? [15:36] if so, point me at 'em [15:36] mgz: yeah, quite a few [15:36] rogpeppe: take a look please: https://codereview.appspot.com/6843109 - since it's trivial one LGTM should suffice, right? [15:36] mgz: look at the juju-core active reviews [15:37] dimitern: it's a general bad assumption [15:37] rogpeppe: will do [15:37] dimitern: will do, in a little while [15:37] rogpeppe: thanks [15:44] rogpeppe, for the web we'll need to have a webserver with the same cert serve up the html so the browser user can ack it, the websocket is considered a subresource, and won't get loaded if the cert isn't already known to the browser. [15:46] rogpeppe: are there any fixes for these tests that aren't nacked by niemeyer? [15:47] brb [15:49] for what it's worth, bzr has a few different test classes, any test that's going to touch disk is given an isolated home directory to work in [15:53] and lp:~rogpeppe/juju-core/141-test-HOME-independence fixes failures. [15:59] mgz: we've moved away from that branch. i've been fixing on a see-it-and-fix-it basis. [16:00] mgz: tbh i'm juggling too many subtly interrelated branches at the moment - i can't remember what goes where! [16:01] it'd be good to have a go-ahead for this trivial: https://codereview.appspot.com/6847091/ [16:01] mgz: ^ [16:02] mgz: and this followup: https://codereview.appspot.com/6782103 [16:05] dimitern: LGTRM [16:05] LGTM [16:05] dimitern: one LGTM is fine for that [16:06] rogpeppe: cool, 10x [16:06] dimitern: if you submit now, i've got two branches that i need to submit in quick succession [16:07] pwd [16:07] rogpeppe: just submitted [16:08] dimitern: thanks [16:08] TheMue, fwereade_, dimitern: please don't submit anything for a minute or so [16:09] rogpeppe Ey, ey [16:09] how does submitting interfere, i'm curious when you're done [16:09] * TheMue leaves now, has a whisky tasting in 50 minutes [16:11] TheMue: enjoy! [16:11] I wish you all a wonderful weekend and thanks for the helpful feedback. [16:11] rogpeppe: I'll do. Slàinte! [16:11] TheMue: np. thanks for going along! [16:11] dimitern: done now. [16:12] dimitern: submitting pulls down a copy of the branch you're submitting against, merges against that, then pushes [16:12] dimitern: so if someone submits in the meantime, you may get a "branches diverged" error [16:13] rogpeppe: I see, good to know [16:13] dimitern: and in this case, one of the branches was breaking trunk, so i didn't want to get into that state [16:18] mgz: i'm just putting together a branch that fixes the various HOME-dependencies [16:19] rogpeppe: ace, will look at those other movey branches too [16:42] niemeyer: oh dear, it look like Settings.Write doesn't work properly in the face of nil keys. i only discovered when running the tests having done "chmod 0 ~/.ssh ~/.juju" [16:43] rogpeppe: Hmm [16:43] niemeyer: am currently working on a fix [16:43] rogpeppe: What's the deal there? [16:43] niemeyer: the nil keys are disappearing [16:43] niemeyer: sorry, the nil values [16:45] niemeyer: it looks ok at a glance, but that's not what i'm seeing [16:47] rogpeppe: Hmm [16:48] niemeyer: yeah, this test fails: http://paste.ubuntu.com/1379890/ [16:50] rogpeppe: I think that's what we represent as lack of value [16:50] niemeyer: maybe i should back out my submits [16:51] niemeyer: again :-( [16:52] rogpeppe: What about using your prior idea of changing the behavior of "" [16:52] aargh [16:52] rogpeppe: Would that work? [16:52] rogpeppe: argh² [16:52] niemeyer: i believe it would :-) [16:53] rogpeppe: I still don't think it's great, but we can't spend weeks on how to load a value from disk [16:53] niemeyer: i know [16:53] niemeyer: we could change the behaviour of "" throughout - it's really just an implementation convenience [16:54] rogpeppe: No, let's please not cascade this further [16:54] rogpeppe: Otherwise Monday we'll be talking about this again [16:54] niemeyer: yup [16:54] niemeyer: i thought i might get tls actually working today too [16:54] niemeyer: (i've got a branch that almost does) [16:54] rogpeppe: Let's keep the current branch mostly unchanged, and try to just adapt the logic so it works with "" [16:55] niemeyer: ok [16:55] niemeyer: could you draft the doc comment for New while i fix the code? [16:57] rogpeppe: I'm on it [16:58] niemeyer: thanks [17:26] hm, is lgtm actually used as a magic string for anything? [17:26] mgz: i don't believe we do that (the go core does, i think) [17:26] pwd [17:26] ~ [17:38] rogpeppe: reviewed those movey branches [17:38] mgz: thank you [17:40] the second one I trust you on the design of, I'd probably have made a cert object that had create and parse constructors and accessors for cert/private key, and made the code create one how they wanted before passing to bootstrap [17:40] rogpeppe: I think the easiest for the moment is to just remove the second to last paragraph [17:41] rogpeppe: It turns out that everything else is still true [17:41] niemeyer: ok. will have a look when i've got these pesky tests passing again [17:41] rogpeppe: We could try to explain, but it feels like it'll make it harder to read and understand than otherwise [17:42] niemeyer: that sounds reasonable [18:05] fwereade_: I know you're on holiday, but you demonstrated interest in talking yesterday. In case you need me, I'm around, but no rush.. we can talk on Monday too [18:09] niemeyer: https://codereview.appspot.com/6854088 [18:09] rogpeppe: Looking [18:09] rogpeppe: Uh, what happened to the rest? [18:10] niemeyer: the rest? [18:10] rogpeppe: I mean, the branch that was already in review? [18:10] niemeyer: it got submitted [18:10] niemeyer: before i realised the problem [18:10] rogpeppe: Oh? [18:10] rogpeppe: Ouch [18:10] rogpeppe: Okay, hopefully that'll fix it.. looking [18:10] niemeyer: i was trying to fix another problem when i found out [18:11] mgz: the above branch should fix your $HOME-dependency problems too [18:11] mgz: i've run all tests successfully with: mkdir /tmp/x; chmod 0 /tmp/x; HOME=/tmp/x go test ./... [18:12] mgz: apart from the state tests, which need a writable .ssh, because ssh *always* writes to your home directory, regardless of the setting of $HOME [18:13] rogpeppe: That change in file bootstrap.go feels bogus [18:13] rogpeppe: I've been looking at the change that puts it back where it should be on every other branch [18:13] niemeyer: i think it might be because i'm using a different version of gofmt [18:14] niemeyer: i.e. the one in tip [18:14] rogpeppe: looking [18:14] rogpeppe: Maybe, but we need to stabilize the situation [18:14] rogpeppe: I've seen *several* branches fixing this [18:14] rogpeppe: We shouldn't change it back [18:14] rogpeppe: Or we'll continue to see it [18:14] niemeyer: yeah. i'll put 1.0.3's version in my bin [18:14] pwd [18:15] rogpeppe: In state_test.go, it's curious that a few lines were not there before [18:15] rogpeppe: THey weren't nil, and now they're being set to "" [18:15] rogpeppe: That means the behavior of the test is actually changing rather than just being moved on the nil/"" situation [18:15] rogpeppe: Was the test borked? [18:16] niemeyer: the test was relying on the fact that ca-private-key did not exist in $home/.juju, yeah [18:17] niemeyer: i smoked out these things by running the tests with an inaccessible home [18:24] rogpeppe: Cool [18:25] rogpeppe: So LGTM [18:25] niemeyer: thanks [18:25] rogpeppe: The gofmt issue is the only detail worth changing before submit [18:25] niemeyer: i've done it [18:25] rogpeppe: I wonder if that's an accident [18:26] rogpeppe: I mean, the gofmt behavior change [18:26] niemeyer: it doesn't look right [18:26] niemeyer: i agree [18:27] rogpeppe: We should report it.. hopefully it can be fixed before the next major [18:27] niemeyer: submitted. [18:27] rogpeppe: Thanks a lot [18:27] niemeyer: davecheney says there are other gofmt changes coming [18:27] niemeyer: in tip [18:27] rogpeppe: Hmm [18:27] niemeyer: i think we should standardise on 1.0.3 [18:27] rogpeppe: Not sure how that affects things.. it feels like an unintended bug [18:28] rogpeppe: (that might be caused by these other changes) [18:28] niemeyer: until 1.1 is released [18:28] rogpeppe: lgtm [18:28] mgz: ta. it's in. i'd like to know if that fixes your problems. [18:29] ah, I should have mentioned, most of the failures are gone [18:29] there's still the odd build thing, and I need to pull to get dimitern's fix [18:30] http://code.google.com/p/go/issues/detail?id=4428 [18:30] mgz: That's related to your comment on the branch too ^ [18:31] niemeyer: right, I saw the discussion here after hitting submit [18:32] right, that's it for me [18:40] niemeyer: here's the branch that implements the --ca-cert-file flag: https://codereview.appspot.com/6842088 [18:41] niemeyer: i've gotta go now, but i'm really hoping you might be able to go through my active reviews before monday :-) it's feely pretty unwieldy atm, although getting that config branch in is a big weight off. [18:44] niemeyer: thanks a lot for bearing with me with these large branches. i'm hoping that there won't be much more in the way of config changes for a while now... [18:44] night all [18:54] rogpeppe: Thanks a lot, and have a nice night and weekend [19:16] niemeyer: and you [19:17] niemeyer: hope your house decoration is going/has gone well [19:19] rogpeppe: It's pretty much done now.. today was the last day of furniture installation, and the cable/internet guys were around too to upgrade the network [19:20] rogpeppe: Slightly less crappy bandwidth now, hopefully video will suck less on our meetings now [19:20] rogpeppe: I can't handle that anymore, so I'm glad it's over