[00:25] chasing another LGTM, https://codereview.appspot.com/7782049/ [00:25] please [01:12] davecheney: looking [01:14] wallyworld: ty [01:14] davecheney: by the looks of it, most of the changes are just mechanical === thumper-lunch is now known as thumper [01:17] davecheney: in provisioner_test, was not using TestingDialTimeout an oversight? [01:26] hmm.... [01:26] coffee... [01:26] I think I should go make one [01:26] bigjools just finished making me a coffee :-D [01:27] he's getting better at it, needs more practice [01:27] no more for you then you cheeky cnut [01:27] but you need the practice === wedgwood is now known as wedgwood_away [03:06] wallyworld: o/ [03:06] hello there [03:07] how's things? [03:10] wallyworld: pretty good, other than being stuck in san antonio :) [03:10] wallyworld: you? [03:12] lifeless: well, switched teams to work on juju. interesting project. learning Go has had it's ups and downs. main issues for me are the lack of expected tools and std lib functionality cf more mature platforms like python etc [03:13] yah [03:13] its hard at the start of a setup like that [03:13] specially when it doesn't eagerly use existing libraries [03:16] lifeless: san antonio? was that pycon or something? [03:16] pycon was in santa clara, I was there [03:17] did a side trip to san antonion to visit rackspace; plane I was meant to fly back to SJC on was bust [03:17] so an extra unplanned night here [03:17] fortunately I had a day leeway before my return flight from SJC to NZ [03:19] hi lifeless [03:19] hi thumper :) [03:20] lifeless: did you see "dongle-gate" [03:21] wtf happened there? sounds like a storm in a tea cup all blown out of proportion [03:21] * thumper invokes barry's 2nd law (http://barry.warsaw.us/software/laws.html) [03:25] wallyworld: I only heard bout it after the conf [04:19] no thumper ? [04:55] http://paste.ubuntu.com/5636105/ [04:55] getting this very consistently [06:12] * davecheney can't figure out why the hook tests are so unreliable [08:00] mornin' all [08:05] rogpeppe, heyhey [08:05] fwereade__: yo! [08:06] rogpeppe, would you take a quick look at https://codereview.appspot.com/7950043/ please? [08:06] fwereade__: looking [08:07] rogpeppe, after that I'd like a quick chat about the params.*Info types [08:07] fwereade__: sounds good [08:13] fwereade__: reviewed [08:14] fwereade__: afk for a little bit, back in a short while [08:21] fwereade__: back [08:23] rogpeppe, I think you're right about the Reset but I got distracted by broken trunk :/ [08:25] fwereade__: that there should be a Flush in the TearDownTest too? [08:25] rogpeppe, yeah, but I need to think about that bit a little more [08:27] fwereade__: shall we have that chat? [08:27] rogpeppe, with you in a sec, would you start a hangout please? [08:28] fwereade__: https://plus.google.com/hangouts/_/405ae790bfb55499116d2961f7e7f2ecc352fb68?authuser=0&hl=en-GB [08:56] fwereade__: any chance you could jump back into that hangout? i just wanted to chat about one other thing [09:12] rogpeppe, sorry, totally didn't see that [09:12] fwereade__: np. i was just wonderin about the suggestion to drop version.Current. [09:13] fwereade__: it seems a bit odd to me - there whole point is so that a given binary can know what version it is [09:13] rogpeppe, I don't think it's sane, I'm responding now [09:13] fwereade__: and report it as such [09:13] fwereade__: ok, cool [09:14] davechen1y: hiya [09:14] davechen1y: can you tell us why dropping version.Current is a good idea? [09:15] rogpeppe: because it is non deterministic [09:16] i can build and test a cli on one series, and run it on another series and it does different things [09:16] davechen1y: but version.Current isn't about testing [09:16] davechen1y: it's about giving a unit the ability to find out what version it's running [09:17] davechen1y: or a juju binary too [09:17] yes, and it is that multitude of roles that is causing the problem [09:17] i think version.Current needs to be replaced by several things which are specific to the role of the caller [09:17] davechen1y, many uses of version.Current are crack but many are not [09:17] davechen1y: how would you find out the current version? [09:18] rogpeppe: sure, that is _1_ use for version.Current [09:18] arguably the correct one [09:18] davechen1y: well, that's why i implemented it... [09:18] but the use of version.Current for 'fuck, I need a version value, but I don't have one, fuckit, version.Current works' [09:18] davechen1y: agreed totally [09:18] throughout the code is causing tim problems [09:18] davechen1y: but that doesn't imply that we should drop version.Current [09:18] and it is _those_ cases we need to fix [09:18] davechen1y: just that we shouldn't use it in tests [09:19] rogpeppe: please don't read too much into my use of the word drop [09:19] by use of the word drop was as a tool expose, then audit all the places where version.Current is being used (mostly incorrectly) [09:19] then, once we've found and fixed them, we can put version.Current back [09:19] this may probably happening within one change proposal [09:20] davechen1y: ah, when i see a subject line like "version.Current must die" and your proposal to "delete version.Current" that sounded pretty unambiguous :-) [09:20] davechen1y: i don't think we need to remove it [09:20] davechen1y: just grep for it [09:20] well, tim wants to kill it, and I want to bisect it [09:20] davechen1y: tim has no proposal for how you would *actually* find the current version [09:21] rogpeppe: i cna't speak for him fully [09:21] but I believe he believes most of those pieces of data come from the environment configuration [09:21] davechen1y: another possibility is to always set version.Current at the start of the tests, to a known value [09:21] i cannot speak further to this [09:21] davechen1y: actually none of those pieces of data come directly from the env config [09:22] rogpeppe: like I said, I cannot explain his plans [09:22] i am not fully aware of them [09:22] davechen1y: np. sorry, i thought you'd discussed it at length. [09:22] this is only what I recally from our conversations [09:22] we'd talked at length about the problme [09:22] ... less so about the solution [09:23] davechen1y: what do you think about just setting version.Current to a known value at the start of tests? [09:24] i don't think tim will accept that [09:24] and that doesn't address the problem that we use it in too many places for too many things [09:25] davechen1y: then testing.DefaultVersion() ? [09:25] rogpeppe: i don't want to speak too much too this [09:25] i don't know tims plans [09:25] davechen1y: ok [09:25] but this is more that just testing [09:25] and is related to the set of cards he has to do with inserting series into start instance [09:26] davechen1y: ok [09:27] davechen1y: thanks for the input [09:27] rogpeppe: related, your issue you raised that looks like it was a crash in the hash map code [09:27] there are two things that might be causing that [09:27] 1, is gc can run concurrently with hashmap insertion [09:27] and corrupt the hash map [09:27] we believe we've fixed those bugs now [09:28] so tip right now shouldn't do that [09:28] assuming that is the cause [09:28] 2. this might actually be reall concurrent hash map mutation [09:28] davechen1y: it died in append - i didn't think it looked like it was in the hashmap code [09:28] davechen1y: but perhaps you were looking at some other aspect of the trace [09:28] but the gdb you got pointed to the hashmap interator [09:28] for sure, things with the new GC are not all beer and skittles [09:29] davechen1y: ah, the second one, yes [09:29] davechen1y: yeah, i'm not really surprised - it's a big change [09:30] davechen1y: i don't know that i entirely trust the disass output though - i was dumping a binary that might have been different from the original [09:30] oh [09:30] well that could point the finger at anything [09:31] davechen1y: well, i *think* i rebuilt the same binary, but i'm not entirely sure [09:31] * davechen1y wishes he'd been able to convince rsc to allow go test -c ./... [09:31] davechen1y: i really tried (perhaps a bit too hard actually) [09:32] davechen1y: workaround: http://paste.ubuntu.com/5636443/ (i call it "gotest-c") [09:32] nah, rsc was right, we're trying to get 1.1 out the door [09:32] not the time for new features [09:32] and we can always just patch our go trees [09:32] or use your workaround [09:33] davechen1y: true 'nuff [09:33] davechen1y: i wish we didn't use cgo at all. then i could be absolutely sure it was a Go bug, not something we're doing wrong at the C boundary [09:38] rogpeppe: good point [09:38] i was tlaking to jam on wed that cross compiling for win32 would be much simpler if we didn't use cgo for goyaml [09:42] davechen1y: if only we could just ditch yaml [09:42] rogpeppe: +1 ;) [09:43] rogpeppe: i like the cut of your jib, sir [09:43] fwereade__: could I trouble you to finish the sentence you started yesterday, https://docs.google.com/a/canonical.com/document/d/1d_b6VTt1bNDCNCrrIVEmEtNIoZi1toGvt5MvVBnxGxw/edit [09:45] davechen1y, done, sorry [09:46] fwereade__: no problem [09:46] if the tree passes i'll do the release tomorrow morning [09:47] davechen1y, cool, thanks [10:13] so, with the latest change all test should pass (just running). then final cleanup and propose. [10:13] TheMue: yes, the tree passes at the moment [10:14] did anyone hear from jam or mgz about a public bucket to push tools' for hp cloud ? [10:21] *hmpf* one fail left [10:27] TheMue: my tree passed, what is failing for you ? [10:38] davechen1y: not the current trunk, only my branch with my changes. ;) they affect several places. [10:40] sadface [10:42] davechen1y: why? [10:42] TheMue, things affecting several places :) [10:43] davechen1y, sorry, I don't know about an hp public bucket [10:43] fwereade__: hehe, yep, always not simple. [10:43] s'ok [10:43] i wrote to jam and mgz [10:43] worse case we can always publish an adendum to the release notes [10:43] davechen1y, sgtm [10:44] *ouch* that panic has been new and definitely not caused by a change. will repeat the test, maybe an intermittent one. [10:46] yip, passes [10:46] TheMue, if so, please make sure it's recorded [10:46] fwereade__: yep [10:56] *very-big-small* all pass [10:57] s/small/smile/ [11:35] lunchtime [12:54] hi rogpeppe [12:54] bac: hiya [12:55] rogpeppe: i've got a question about charm Meta -- you the right guy? [12:55] bac: ... maybe :-) [12:56] rogpeppe: for charms that don't provide a 'limit' the default is 0, except for logging, in the provides section the limit is set to 1. i'm trying to figure out if that is correct and a general rule [12:57] so if you look at testing/repo/series/logging/metadata.yaml you'll see no limit set for the 'provides' section. but those values are being returned as 1 [12:57] bac: take a look at charm/meta.go how the yaml gets parsed and default values filled in [12:57] dimitern: i have and i didn't find my answer. [12:58] bac: hmm, i wasn't previously aware of the "limit" attribute [12:58] dimitern: i don't, however, understand the 'coerce' bits [12:59] bac: Coerce is kinda clever [13:00] fwereade__: does limit limit the number of units that can join a particular relation? [13:00] bac: judging from Coerce and the schema (ifaceSchema) limit can be either nil (empty, like "limit: ") or an int [13:01] rogpeppe, I *think* that limit is meant to mean how many services that relation can be established with [13:01] fwereade__: i think that's what i meant actually [13:01] rogpeppe, but to the best of my knowledge there has never been code that actually interprets it [13:01] fwereade__: ok, so bac's question is kinda moot [13:02] rogpeppe, yeah [13:02] rogpeppe, there's a bug for it somewhere [13:02] rogpeppe, and also for, er, is it "required"? [13:02] fwereade__: yeah, that was weird for me when i saw it as well [13:02] rogpeppe, some other meta relation field anyway [13:03] fwereade__: i'm trying to think of a scenario where it might be useful [13:03] rogpeppe: it is moot, but right now i have to change my expected results to match something i don't really expect (1 vs 0) and that makes me uncomfortable. [13:03] bac: ha, i see. [13:03] rogpeppe, it doesn't make much sense to have wordpress talking to two different databases [13:03] if that is the case, i'd rather just remove it from the results i return. [13:03] bac, +1 to not representing it in the gui [13:04] fwereade__: hmm, i didn't realise you could have a "requires" relation with more than one service [13:04] fwereade__: ok, so remove it from the results i send back. sound good to me. [13:04] bac, sgtm [13:05] fwereade__: so currently, you can do: juju deploy wordpress mysql1 mysql2; juju add-relation wordpress mysql1; juju add-relation wordpress mysql2 and it'll "work" ? [13:06] rogpeppe, I am not aware of anything that would prevent it [13:06] mramm: i see you added the on call reviewer schedule to the calendar, but how about what we discussed - having 2 people each day (distributed across time zones)? [13:06] fwereade__: hmm [13:06] dimitern, I think we decided to go with the simplest thing for now [13:07] fwereade__: ah, ok then [13:08] rogpeppe, TheMue, mramm: I have to pick up laura today, I won't make the kanban meeting I'm afraid [13:08] fwereade__: ok [13:08] fwereade__: ok, thx for the info. === wedgwood_away is now known as wedgwood === bcsaller_ is now known as bcsaller [13:54] dimitern: I don't know how to do time zone distribution in a reasonable way -- there are basically 2 zones, EU and AU, with 5 and 3 people in them, plus john who is sort of in the middle. [13:54] and I thought we had decided to just do 1 person for now [13:54] but if we need time-zone spread, we can do that [13:54] mramm: well, we can have 1 guy around AU and 1 around EU each day maybe? [13:55] well, that means that the AU guys have to do it more than 1x week [13:55] mramm: for now it's ok, but we should think about it if it proves insufficient (slow reviews e.g.) [13:55] right [13:55] we will improve as needed === benji___ is now known as benji [14:36] rogpeppe, dimitern: if you've got time to do a review i'd appreciate it: https://codereview.appspot.com/7668045 [14:36] bac: i'll take a look in a bit [14:36] thanks! [14:37] bac: me too [14:37] \o/ [14:54] bac: reviewed [14:54] thanks [14:58] TheMue, would you explain the high-level thinking behind the various funcs for manipulating home/juju_home? [14:58] TheMue, I'm having some difficulty forming a picture of what gets called when and why [15:14] fwereade__: latest in the version.Current saga: tests fail when run on quantal 386 [15:15] fwereade__: because imagesData doesn't have any image for that combination [15:15] rogpeppe, I'm not surprised, I'm starting to think we basically shouldn't ever be using real series [15:15] fwereade__: in testing, yeah. [15:16] rogpeppe, indeed :) [15:16] fwereade__: except live tests, i guess [15:16] rogpeppe, true, but they need distinct config anyway [15:17] fwereade__: yup [15:18] TheMue, ping [15:19] fwereade__: just fetched a tea ;) and wanted to answer your question [15:19] TheMue, heyhey, I was just looking at the JUJU_HOME stuff [15:19] fwereade__: Init() is to init the juju home based on $JUJU_HOME or $HOME [15:20] fwereade__: JujuHome() returns it [15:20] TheMue, the big question in my mind is why we're calling Init() in test setup [15:20] JujuHomePath() allows to build a path to a file/dir in juju home [15:21] fwereade__: because otherwise some tests are failing (with a panic) when they retrieve the juju home somewhere in a nested function [15:21] fwereade__: I first tried it w/o any init and then digged down step by step [15:22] TheMue, but why would we want to require that HOME or JUJU_HOME be set in order to run tests? [15:22] fwereade__: so if a test tests Foo(), which calls Bar() and that Yadda() and Yadda() references to the juju home, you simply need it [15:22] TheMue, no [15:22] TheMue, we should not need any magic env vars set just to run the tests [15:23] fwereade__: we do, already now [15:23] TheMue, those? or others? [15:23] fwereade__: we have many code, which relies on $HOME [15:23] fwereade__: and those tests would fail w/o a $HOME too [15:23] TheMue, I thought this branch was replacing that? [15:24] fwereade__: this branch now replaces the static init() [15:25] fwereade__: but the code using $HOME is nested [15:25] fwereade__: we said JujuHome() shall panic, if $HOME or $JUJU_HOME are not set [15:25] TheMue, so what? surely the tests should be manipulating exactly one thing -- the value of jujuHome [15:26] fwereade__: so the replacement of the old direct code with the usage of the new API would panic [15:26] TheMue, and we have SetTestJujuHome for that [15:27] TheMue, but you have made STJH panic if Init hasn't been called, which STM to be missing the point [15:27] fwereade__: ok, sounds reasonable [15:27] TheMue, also, what's the deal with origJujuHome? [15:28] fwereade__: but still there is code that is init statically in tests which relies on a juju home (or on $HOME in the past) [15:28] TheMue, can you give me an example of when we need to call Init in a test (that isn't testing Init? ;p) [15:28] fwereade__: it allows to restore it, even if $JUJU_HOME as env var has been changed inside the tests [15:29] TheMue, well, kinda [15:29] fwereade__: one moment, cleaned up my editor, have to find the place [15:29] TheMue, but it stops you being able to nest `defer STJH(STJH(c.MkDir()))` [15:30] TheMue, if you do that when it's already been done somewhere else, things will act weird [15:31] fwereade__: https://codereview.appspot.com/7923043/diff/12001/environs/cloudinit/cloudinit_test.go?column_width=120 line 26ff [15:32] TheMue, I see no reason to call Init there [15:32] fwereade__: why do you want to nest setting a test home? [15:32] TheMue, ah, ok, I do -- Init and SetTestJujuHome are equally bad there [15:33] fwereade__: ok, setting home should work too, yes. inside of config.New() today $HOME/.juju is accessed [15:33] TheMue, because when you see that line, you think it's bug-free in and of itself, but actually it's not... or maybe it is, but it becomes really hard to tell when you have all these different mechanisms crossing over [15:34] fwereade__: sorry, can't follow your last sentence [15:34] TheMue, sorry [15:35] TheMue, I'm saying that there is too much surprising action at a distance for me to follow [15:35] fwereade__: but yes, I can follow your idea of using STJH() also for an initializing in tests. here I thought too short [15:36] TheMue, but I don't seem to have communicated why it is wrong to call Init in tests? [15:36] TheMue, I wouldn't have STJH in code but I'm not typing it out every time in irc ;) [15:37] TheMue, you should not be calling Init in tests because it's making a global state change that cannot be restored cannot be restored [15:37] fwereade__: what I meant is, that STJH shouldn't rely on an Init before [15:38] TheMue, ah, got you [15:38] fwereade__: it can be restored [15:38] fwereade__: that's why ...Orig is only set once and never changed again [15:39] TheMue, original correct state -- with jujuHome="" -- *cannot* be restored [15:39] TheMue, it will panic if you try [15:39] TheMue, if you put in Init in one test, other tests in the package might or might not run Inited, dependingon circumstance [15:40] TheMue, am I making sense to you? [15:41] fwereade__: would have like to see it painted on a whiteboard. *lol* [15:41] fwereade__: but yes, Restore... relies on Init before [15:41] fwereade__: and that's the failure [15:42] TheMue, what's the actual use case for Restore... anyway? [15:42] TheMue, can't we just just STJH throughout? [15:43] fwereade__: hmm, would be possible. but I would like a mechanism like in FakeHome more. an own type with a Restore method [15:43] fwereade__: so the call would be defer STJH("foo").Restore() [15:43] TheMue, that would be fine too, my problem is that we have a load of different ways to do basically the same thing [15:44] fwereade__: yes, we have, the problem with a universal programming language ;) but rogers idea is very elegant [15:45] TheMue, in fact that would be nice, because you can just stick it in a field with clear intent [15:45] TheMue, sorry, which idea? [15:45] fwereade__: the idea of an own type (based on a string) with a Restore method [15:46] TheMue, we have two fakeHome types already I think, I would be hoping to merge those rather than to create a new one:) [15:49] fwereade__: i left them almost untouched for now, but i think they may be merged and better integrated into the juju home idea in future (refactoring card for post 13.04) [15:49] TheMue, but the main point is that config.Init() must not be called in tests, except for those testing itself [15:49] fwereade__: yep, got you [16:10] TheMue, long review sent I'm afraid [16:10] fwereade__: ok, thx. will handle it with care. ;) [16:12] aww poo, self-evaluation [16:12] guess I won't be getting anything else done today [16:15] fwereade__: btw, the cachePath change to $JUJU_HOME/cache works because of the setting of $JUJU_HOME in Init if it not yet set [16:15] fwereade__: oh bugger, that [16:15] * TheMue done this evaluation stuff this morning [16:15] TheMue, but that doesn't happen [16:16] fwereade__: what doesn't happen? [16:16] TheMue, setting $JUJU_HOME in Init [16:17] TheMue, which is a good thing, because I can think of no good reason to do so [16:17] TheMue, but it does mean the charm store will only work in very specialized circumstances [16:17] fwereade__: i'm afraid i got horribly diverted today by a feasibility study of whether it was possible to test our transactions thoroughly. http://play.golang.org/p/iyfOvjcWR1 [16:17] fwereade__: answer: it would be possible, but quite expensive. [16:17] fwereade__: argh, sorry, missed it when changing the code. had it in there before. [16:18] rogpeppe, that's cool, but, yeah, not lightweight [16:18] fwereade__: if our ops on mongo weren't so slow, it might be feasible [16:19] fwereade__: the nice thing is it can test arbitrary transactions together in a thorough way [16:20] rogpeppe, that is very nice, indeed [16:21] fwereade__: it's something that we might consider writing tests for, even if they took days to run, just so we can have some kind of assurance. [16:21] rogpeppe, yeah, I would be very keen on future work in this direction [16:22] fwereade__: i'll save it for later :-) [16:27] fwereade__: the only major assumption is that when an operation generates a transaction, it's a deterministic function of the state it's previously seen. === deryck is now known as deryck[lunch] [17:44] happy weekends everyone [17:44] fwereade__: you too! :) [17:52] fwereade__: any you! [17:52] s/any/and === deryck[lunch] is now known as deryck [18:48] happy weekends all! [18:53] rogpeppe: same to you :) === wedgwood is now known as wedgwood_away