[00:32] gocheck fans [00:32] is there a method to check []string, contains, string ? [01:08] axw: partial review done, gotta go get coffee, wil finish soon [01:11] wallyworld: thank you [01:14] davecheney: I've done jujud/agent now [01:14] davecheney: I don't think there is a checker for that... [01:14] yeah, i wrote one myself [01:14] davecheney: jc.Contains is a substring contains IIRC [01:15] urgh [01:25] davecheney: do you know of a bug number for the gccgo problem comparing interface to struct? [01:35] * thumper sighs... [01:36] hard to make the tests pass on my branch for python-jujuclient when the tests don't even pass on trunk... [01:39] meh, blocked by https://bugs.launchpad.net/python-jujuclient/+bug/1465084 [01:39] Bug #1465084: Tests fail with local provider and Juju 1.23.3 [01:48] thumper: what bug is that ? [01:53] thumper: ready for 1:1 [01:53] ? [02:02] * thumper coming [02:10] thumper: https://github.com/go-check/check/issues/43 [02:16] thumper: https://github.com/juju/juju/pull/2556 [02:19] wallyworld: I've made a small change, about to push. added another case to the volumes which are removed on machine removal: unprovisioned machine-scoped [02:28] axw: ok, will look [03:56] davecheney: could you point Chris on https://github.com/go-check/check/pull/35 in the right direction of the sync call needed for the other read location? === anthonyf is now known as Guest37830 [04:43] axw, davecheney: defers are last in first out right? [04:43] yep [04:44] thumper: ^ [04:44] cheers [04:59] thumper: will do [04:59] thumper: looks like your jujud/agent fix, didnt [05:00] * thumper grunts [05:00] * thumper looks [05:00] man... [05:01] hmm... [05:01] * thumper interrupts his current work [05:03] haha [05:03] ha [05:03] ha [05:03] hmm [05:03] not sure why the firewaller isn't starting... [05:03] but I know why the test is timing out at 20 minutes [05:03] it is because the rsyslog worker restarts every x seconds [05:03] thumper: http://paste.ubuntu.com/11717800/ [05:03] because it can [05:03] current state of play [05:04] there are new races in there [05:04] can't write to a dir [05:12] Bug #1465115 opened: api: data race in test [05:17] thumper: https://github.com/juju/juju/blob/master/apiserver/apiserver.go#L187 [05:18] WTF [05:18] * thumper looks [05:18] NewServer prints out the address of the listneing socket [05:18] then ignores it [05:18] and passes new address to the srv type !?!?? [05:18] WTFBBQ [05:19] changes local IP address for localhost? [05:20] yes [05:20] change tghe listening address, the same port on localhost [05:20] i can't even, literally, even [05:32] thumper: [05:32] https://github.com/juju/juju/blob/master/apiserver/server_test.go#L114 [05:32] this is nuts [05:33] we mask the address the apiserver is listening on [05:33] so that we can reliably construct urls pointing to localhost [05:33] what's the point of constructing a URL to localhost ?!?! [05:34] the comment says because it resolves nicely with IPv4 and IPv6 [05:36] it's wrong [05:37] nobody can be using srv.Addr [05:38] this is another horrible string in place of typed objects problem [05:38] hostname, portString, err := net.SplitHostPort(srv.Addr()) [05:38] ^ treating the address like a string [05:39] hmm.. [05:39] * thumper has to go and make dinner now [05:40] also, the apiserver tests sleep for a minute at the end of the test [05:40] because, well, basically, fy [06:12] ---------------------------------------------------------------------- [06:12] PANIC: action_test.go:1: unitSuite.TearDownTest [06:12] ... Panic: local error: bad record MAC (PC=0x414746) [06:12] /usr/lib/go/src/pkg/runtime/panic.c:248 in panic [06:12] /home/ubuntu/juju-core_1.25-alpha1/src/github.com/juju/testing/mgo.go:559 in resetAdminPasswordAndFetchDBNames [06:12] /home/ubuntu/juju-core_1.25-alpha1/src/github.com/juju/testing/mgo.go:503 in MgoInstance.Reset [06:12] /home/ubuntu/juju-core_1.25-alpha1/src/github.com/juju/testing/mgo.go:605 in MgoSuite.TearDownTest [06:12] /home/ubuntu/juju-core_1.25-alpha1/src/github.com/juju/juju/juju/testing/conn.go:113 in JujuConnSuite.TearDownTest [06:12] /usr/lib/go/src/pkg/reflect/value.go:345 in Value.Call [06:12] /usr/lib/go/src/pkg/runtime/proc.c:1394 in goexit [06:12] ---------------------------------------------------------------------- [06:12] PANIC: unit_test.go:317: unitSuite.TestPublicAddress [06:12] ffs [07:06] jam, o/ sorry for being late, omw [07:06] dimitern: actually I was looking for you [07:07] it looks like Mark moved the spec meeting to today [07:07] so I've got a conflict [07:07] I see [07:07] ok, so no meeting then? [07:07] :) [07:07] dimitern: I'm happy to reschedule as it seems like we've been missing eachother. But I should also see you at standup I guess. [07:07] jam, indeed [07:08] dimitern: but yes, enjoy your breakfast [07:14] jam, thanks :) [07:27] Bug #1465157 opened: uniter/filter: FilterSuite.TestUnitRemoval failure in CI [07:57] wallyworld: ping [07:58] hi [07:58] thanks for doc comments [07:58] i need to rework examples etc to match changes [08:03] wallyworld: Did you see the changes on the draft spec ? [08:03] wallyworld: mark and I met today and we had some thoughts on the syntax [08:03] jam: yeah, i already acted on them [08:03] wallyworld: which I'm happy to discuss with you via whatever mechanism you prefer :) [08:03] i've got to update doc examples to match [08:04] wallyworld: k, I might have been messing with your changes then, cause I was trying to put them into the use case spec as suggestions [08:04] ah, i haven't updated the se cases yet [08:04] that's on my todo list [08:04] jam: about to go eat dinner, i'll ping you later on [08:05] wallyworld: k [08:36] dimitern: now you're frozen [08:56] jam: did you want to talk about CLI syntax? [09:02] dimitern: ping [09:02] voidspace, omw [09:06] fwereade: hey, are you able to look at and comment on bug 1464470 [09:06] Bug #1464470: A subordinate charm hook scheduled to run(but waiting for the principal charm hook to release the lock) goes to an error state after the principal charm hook triggers a reboot. <1.24> <1.25>

1.24:Triaged> [09:06] some initial analysis has been done [09:06] needs a sme :-) [09:07] wallyworld, right, I need to think a bit more about that [09:07] sure np :-) [09:08] wallyworld, I think the fix is correct in spirit but I haven't figured out all the actual consequences [09:08] yup [09:09] dooferlad: https://github.com/juju/juju/compare/devices-api-maas...voidspace:generate-macs [09:13] dooferlad: I can talk to you about that after standup [09:13] voidspace: thanks - sounds good [09:51] voidspace, o/ [09:56] dimitern: omw [09:57] dimitern: there [12:44] * Mmike grabs food === anthonyf is now known as Guest35437 === ashipika1 is now known as ashipika [13:32] axw: any chance you're around? :) === lazyPower|eow is now known as lazyPower === kadams54 is now known as kadams54-away [14:19] Bug #1403955 opened: DHCP's "Option interface-mtu 9000" is being ignored on bridge interface br0

[14:31] Bug #1465307 opened: 1.24.0: Lots of "agent is lost, sorry!" messages

=== kadams54-away is now known as kadams54 [15:01] Bug #1465317 opened: Wily builds fail: panic: osVersion reported an error: Could not determine series

[15:03] voidspace, you have a review btw [15:03] dimitern: cool [15:03] dimitern: I realised there's a bit of code missing though [15:04] dimitern: which probably also means missing tests :-/ [15:04] voidspace, oh really? [15:04] dimitern: I need to populate NetworkInfo for kvm broker as well as lxc broker... [15:04] voidspace, well, I trust your judgment then :) [15:04] voidspace, oh, of course [15:04] dimitern: the important code is in configureConatinerNetworking which is used by both lxc and kvm brokers, and just happens to live in lxc-broker.go [15:05] so I was only manually testing that mac addresses were used by the lxc broker [15:05] when I discovered that NetworkInfo was empty [15:05] it's a one line fix, but also needs some test fixes too I think [15:05] ah no, everything passes [15:05] I'd better add one more test for this though [15:05] dimitern: ok, will look at your review - thanks [15:06] dimitern: wow, a Ship It! first time [15:06] dimitern: shame it wasn't complete ;-) [15:08] voidspace, ") [15:08] voidspace, :) even [15:19] dimitern: updated [15:39] i am consistently seeing test failures in worker/provisioner (TestStartInstanceWithStorage and TestStartInstanceLoopMountsDisallowed) [15:39] can anyone get those tests to pass, or is it just me? [15:40] this is on juju-core tip, BTW [15:40] rogpeppe1: there's a data race I believe [15:40] voidspace: really? [15:40] rogpeppe1: normally those tests just pass silently [15:40] rogpeppe1: yes [15:41] voidspace: do you know what the race is? [15:41] rogpeppe1: startInstance in lxcBrokerSuite calls fakeAPI methods [15:41] rogpeppe1: these have an assert in them [15:41] voidspace: i've removed the assert [15:41] rogpeppe1: ah, that's why you have the problem then [15:41] rogpeppe1: both those tests pass for me btw [15:41] voidspace: (it always failed anyway) [15:42] natefinch: interesting. [15:42] the assert failing was halting the test prematurely [15:42] natefinch: they will pass until you remove the assert in the fake API [15:42] rogpeppe1: this branch fixes it [15:42] voidspace: i've removed the assert [15:42] voidspace: ah, that makes sense [15:42] dammit [15:42] rogpeppe1: https://github.com/juju/juju/pull/2565 [15:43] voidspace: well, rogpeppe1 asked if it passed on tip, and the answer is that it passes on tip for me... not with any changes :) [15:43] rogpeppe1: but that's merging onto a feature branch [15:43] natefinch: right [15:43] fair enough :-) [15:43] voidspace: it would be great to submit it is a separate fix if poss [15:43] rogpeppe1: I believe that dooferlad is extracting the data race fix as a separate fix and merging it to trunk [15:43] voidspace: ah, it's merging already [15:43] rogpeppe1: so "in progress" [15:43] voidspace: it's not really a race [15:44] voidspace: more like a bug in gocheck [15:44] rogpeppe1: ok, sounds plausible [15:44] voidspace: ok, so i'll ignore the test failures for the time being. hopefully your branch will land. [15:45] voidspace: what's the *gitjujutesting.Stub field for in the fakeAPI struct? [15:46] rogpeppe1: it's Stub that provides MethodCall, NextErr and SetErrors [15:47] rogpeppe1: and CheckCalls [15:47] voidspace: ah, so the required interface has grown, i see [15:47] rogpeppe1: well, Stub is an implementation not an interface [15:48] voidspace: those methods weren't provided before by fakeAPI, so i can only presume that the required interface that it's implementing has grown [15:48] rogpeppe1: nope [15:48] rogpeppe1: Stub provides useful mocking methods for fakeAPI [15:48] voidspace: hmm, i must be missing something then [15:49] rogpeppe1: nothing to do with the API that fakeAPI provides [15:49] voidspace: ok, i must have missed where they're used [15:49] rogpeppe1: they're not required for fakeAPI they're useful for testing [15:49] rogpeppe1: grep through that diff to see them in use [15:49] rogpeppe1: CheckCalls uses the results of MethodCall [15:50] rogpeppe1: NextErr uses the errors set by SetErrors [15:50] voidspace: i think it's somewhat misleading to embed Stub as an anonymous field [15:50] voidspace: it makes it look like it's being used to expose methods outside of fakeAPI [15:50] rogpeppe1: what do you mean by "outside"? [15:51] rogpeppe1: it's the intended use of Stub I believe [15:51] voidspace: the whole point of fakeAPI is to fulfil the provisioner.APICalls interface [15:51] voidspace: by embedding Stub, you make it look as if that is helping to fulfil that interface [15:51] rogpeppe1: right, and Stub is a helper to do that [15:52] rogpeppe1: the name is a big clue :-) [15:52] voidspace: whereas it's actually just an implementation detail *inside* fakeAPI [15:52] as well as how it's used [15:53] voidspace: i don't understand - none of the methods required by provisioner.APICalls are implemented by Stub [15:53] voidspace: so the fact that it's embedded isn't helping at all [15:53] rogpeppe1: it's a helper for writing stubs [15:53] it doesn't take much beyond knowing [anything about] the API it provides to realise what it's for [15:54] voidspace: generally we embed fields for a reason, usually because we need the methods they implement in the method set of the object they're embedded in [15:54] rogpeppe1: Stub is regularly embedded in fakes like this [15:54] voidspace: in this case, it seems to be embedded just to avoid an explicit field name reference [15:54] and knowing what Stub doesn't it isn't reasonable to think that Stub actually provides any methods of the external API that the fake object provides [15:55] so you seem to be making a purist point rather than a practical one [15:55] voidspace: it would be much more obvious that it wasn't involved if it wasn't embedded [15:55] voidspace: i'm saying that it makes the code less readable [15:55] and I'm saying I disagree [15:56] if it is a confusion it can only be if you haven't seen Stub before, and even then only momentarily [15:56] and this is how Stub is used in many places in our code [15:56] presumably you're the first to complain [15:58] voidspace: the Stub documentation can't agree with itself whether it's supposed to be embedded or not [15:58] voidspace: i see that it's used in other places like that, and i think it's a mistake [15:59] voidspace: i think it makes a big difference whether the method set of an object is immediately seeable or not. [16:00] voidspace: might i ask *why* you think it's a good idea to embed it, rather than using a named field? [16:00] I don't think it's a bad idea [16:00] and given that fake objects provide a specific interface (that being their point) [16:00] and Stub objects provide a different, very specific, set of methods [16:01] I don't think there's any reasonable possibility for confusion once you know what Stub does [16:01] voidspace: but what's the advantage of embedding? [16:01] voidspace: it can only be that it's slightly less code to use, i guess [16:01] I don't see any advantage to mandating *not embediding* [16:01] which is the direction you want to go [16:01] right, slightly extra effort [16:02] voidspace: embedding imposes greater cognitive load on the reader of the code [16:02] I disagree [16:02] as I've already stated [16:02] voidspace: "extra effort" being as hard as ".stub" [16:03] voidspace: you've said "once you know what Stub does" - that's what i mean by extra cognitive load [16:03] extra cognitive load being as hard as "that's a stub method" [16:03] to understand the code you *need* to know what it does [16:03] you can't get away from that! [16:04] voidspace: i don't recognise the term "stub" from normal Go idioms [16:04] it's a programming term [16:04] :-) [16:04] language agnostic [16:04] voidspace: it's a terrible name - it means nothing really [16:04] getting you a link to the classic martin fowler paper [16:05] it's the generally used one [16:05] http://martinfowler.com/articles/mocksArentStubs.html [16:05] voidspace: it would be better as "APIStub" perhaps [16:05] see that article [16:06] Stub itself can be used for any interface in juju - not just for APIs [16:06] voidspace: by the terminology of that article, i don't think it's either a mock or a stub [16:06] voidspace: i think it's a helper [16:06] yeah, a helper for writing stubs [16:06] voidspace: sure [16:06] I didn't write it or name it either by the way [16:07] voidspace: i think it's confusing to call it a stub when it's not, especially as "testing.Stub" tells you nothing at all about what it might be useful for [16:08] rogpeppe1: ok [16:08] voidspace: and i think that embedding is conflating the testing.Stub helper with the stub itself in an unhelpful way [16:08] voidspace: i'll stop now. you weren't responsible :) [16:08] rogpeppe1: :-) [16:09] voidspace: it feels like something implemented by someone that's still thinking in an inheritance-based way, not someone that actually knows Go well [16:09] rogpeppe1: I don't object to a named field (with the exception I guess if that has to be a public field if you're writing re-usable stubs) [16:09] rogpeppe1: however I don't philosphically disagree with embedding either [16:10] it certainly works fine not embedded - at the cost of an extra level of indirection for every use [16:10] voidspace: if i was writing a wrapper around Stub that just added some functionality, *then* embedding would be appropriate [16:11] voidspace: that's not a runtime cost, of course [16:11] I don't care about runtime cost until it's proven to be an issue [16:11] voidspace: +1 [16:11] I'm talking about cognitive overhead for the reader of the code ;-) [16:11] voidspace: for me, the "cost" of an extra level of indirection is actually a net benefit to the reader of the code [16:11] deliberate winky-smiley as I'm provoking you... [16:12] voidspace: because you know exactly which field is being routed to [16:12] rogpeppe1: I understand what you're saying [16:13] voidspace: BTW you could use Stub as a value in the struct - no need to declare it as a pointer [16:15] rogpeppe1: that would probably simplify it a bit [16:16] rogpeppe1: just to show that I'm not utterly recalcitrant I'll change it to a field, and a value rather than a pointer [16:16] rogpeppe1: in a follow up [16:16] rogpeppe1: that branch landed - but only on a feature branch, not on trunk [16:16] voidspace: thanks, that would be appreciated [16:16] rogpeppe1: dooferlad is working on porting the relevant bits to trunk [16:16] voidspace: oh bugger [16:16] voidspace: i just pulled, hoping that would fix my tests [16:17] rogpeppe1: so you can have exactly the same conversation with him... [16:17] rogpeppe1: afraid not, unless you pulled our feature branch! [16:17] rogpeppe1: I'll chat to dooferlad about it, I presume he's not around *right now* or he would already have popped up [16:17] dooferlad: ping if you're around [16:18] rogpeppe1: (I did talk to him this morning about porting these changes across - so I know he's working on it) [16:18] voidspace: perhaps this one fix could be ported separately [16:18] rogpeppe1: yep [16:18] rogpeppe1: he may already be part the way through it [16:19] rogpeppe1: it involves changes to lxc-broker_test.go *and* kvm-broker_test.go (as kvm broker uses the fakeAPI as well) [16:19] rogpeppe1: so if he's started it would be quicker to get that in than for me to unpick the branch I've just landed [16:19] rogpeppe1: but if he hasn't done it I can do it tomorrow (morning hopefully) [16:24] voidspace, rogpeppe1: Hi, was just AFK for 10 minutes saying hello to the family since they just got home. I am just about to push my changes after a quick readthrough. They pass all the tests I have done. [16:24] voidspace: thanks. it would be nice to have juju-core tests passing locally [16:42] rogpeppe1, voidspace: http://reviews.vapour.ws/r/1936/ [16:42] there will be a 1.4 change landing soon [16:43] s/landing/being proposed/ [16:43] dooferlad: 1.4 version of what? Go? [16:43] dooferlad: looking [16:45] rogpeppe1: juju [16:46] dooferlad: ah, i've totally lost track of juju version numbering :) [16:46] dooferlad: aren't we at ~1.25 ? [16:47] dooferlad: or is this a version for some different aspect of juju? [16:47] dooferlad: I presume you mean 1.24... [16:47] rogpeppe1: you are correct [16:47] rogpeppe1: oh, 1.24. Just being distracted by a tiny cute person. Sorry! [16:47] voidspace: ah, that really confused me! [16:52] OK, need to call it a day. The fix backporting involves a little more than a cherry pick and fixing up the juju/testing version so I will get it done tomorrow morning. [16:52] dooferlad: you have a review [16:52] voidspace: thanks [16:53] dooferlad: mostly trivial - but there's one test that I think might be wrong [16:53] voidspace: will get to it tomorrow first thing. Thanks for taking a look! [16:53] dooferlad: cool [16:57] wow, cmd/juju tests now taking 9m to run! [17:36] rogpeppe1: is that better or worse than they used to be for you? (I'm hoping worse) [17:36] natefinch: i'm pretty sure it's quite a bit worse [17:36] actually, I guess better is better.... but still. 9m sucks [17:36] natefinch: i remember 5 minutes, but 9?! [17:36] rogpeppe1: is that on tip? [17:36] natefinch: yeah [17:37] natefinch: 9m for one package| [17:37] ! [17:38] natefinch: i'm off now, back on wed [17:38] rogpeppe1: have fun! [17:38] natefinch: will do [17:41] rogpeppe1: fwiw, go test in cmd/juju runs in 2m35s on my machine on tip [17:41] note that's go test, not go test ./... if that makes a difference [17:42] rogpeppe1: they time out on my machine [17:42] voidspace: doh [17:42] yep :-) [17:43] natefinch: 2m35! I'm on vivid [17:43] I wonder if they're worse on vivid [17:43] voidspace: I'm on utopic... it might be my SSD, it's a Samsung 850 pro, itsuper fast. [17:44] s/itsuper/it's super/ [17:45] voidspace: I noticed davecheney complaining about the time tests took, and his numbers were like 30 mins for the whole suite [17:45] voidspace: I'm running go 1.3.3 right now (just happens to be what I last switched to), and I run gomaxprocs=8. Not sure if any of that makes a big difference or not [17:48] natefinch: I think my SSD is an 840 - not going to be a *great* deal of difference I don't think [17:48] I'll try them on my laptop [17:48] natefinch: I'm running go 1.2.1 maybe that hurts === kadams54 is now known as kadams54-away [17:50] voidspace: newer versions are definitely faster, though I wouldn't think they'd be that much faster [17:50] gomaxprocs wouldn't affect the timing of one package, as that will be a single process [17:51] voidspace: good point [17:52] natefinch: if it makes it faster enough to not timeout that would be progress for me... [17:52] updating to 1.3 [17:53] voidspace: you're on an XPS 15, too, right? [17:53] natefinch: this is on my desktop [17:53] voidspace: which should be even faster, presumably [17:54] natefinch: yeah, but not much [17:54] trying now with go 1.3.3 [17:54] natefinch: I wonder if vivid is the difference [17:54] natefinch: my laptop is utopic [17:54] natefinch: I'll try on that [18:37] man running tests on a bar drives all kinds of odd looks to your screen [18:37] heh [18:41] * perrito666 deploys a lot of things on a bar, will start hearing angry people in 3, 2, 1 .... [19:11] Brb [19:20] perrito666: natefinch: cherylj: wwitzel3: i need a volunteer to look at bug 1465307 [19:20] Bug #1465307: 1.24.0: Lots of "agent is lost, sorry!" messages

[19:25]