/srv/irclogs.ubuntu.com/2013/03/22/#juju-dev.txt

davecheneychasing another LGTM, https://codereview.appspot.com/7782049/00:25
davecheneyplease00:25
wallyworlddavecheney: looking01:12
davecheneywallyworld: ty01:14
wallyworlddavecheney: by the looks of it, most of the changes are just mechanical01:14
=== thumper-lunch is now known as thumper
wallyworlddavecheney: in provisioner_test, was not using TestingDialTimeout an oversight?01:17
thumperhmm....01:26
thumpercoffee...01:26
thumperI think I should go make one01:26
wallyworldbigjools just finished making me a coffee :-D01:26
wallyworldhe's getting better at it, needs more practice01:27
bigjoolsno more for you then you cheeky cnut01:27
wallyworldbut you need the practice01:27
=== wedgwood is now known as wedgwood_away
lifelesswallyworld: o/03:06
wallyworldhello there03:06
wallyworldhow's things?03:07
lifelesswallyworld: pretty good, other than being stuck in san antonio :)03:10
lifelesswallyworld: you?03:10
wallyworldlifeless: 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 etc03:12
lifelessyah03:13
lifelessits hard at the start of a setup like that03:13
lifelessspecially when it doesn't eagerly use existing libraries03:13
wallyworldlifeless: san antonio? was that pycon or something?03:16
lifelesspycon was in santa clara, I was there03:16
lifelessdid a side trip to san antonion to visit rackspace; plane I was meant to fly back to SJC on was bust03:17
lifelessso an extra unplanned night here03:17
lifelessfortunately I had a day leeway before my return flight from SJC to NZ03:17
thumperhi lifeless03:19
lifelesshi thumper :)03:19
wallyworldlifeless: did you see "dongle-gate"03:20
wallyworldwtf happened there? sounds like a storm in a tea cup all blown out of proportion03:21
* thumper invokes barry's 2nd law (http://barry.warsaw.us/software/laws.html)03:21
lifelesswallyworld: I only heard bout it after the conf03:25
davecheneyno thumper ?04:19
davecheneyhttp://paste.ubuntu.com/5636105/04:55
davecheneygetting this very consistently04:55
* davecheney can't figure out why the hook tests are so unreliable06:12
rogpeppemornin' all08:00
fwereade__rogpeppe, heyhey08:05
rogpeppefwereade__: yo!08:05
fwereade__rogpeppe, would you take a quick look at https://codereview.appspot.com/7950043/ please?08:06
rogpeppefwereade__: looking08:06
fwereade__rogpeppe, after that I'd like a quick chat about the params.*Info types08:07
rogpeppefwereade__: sounds good08:07
rogpeppefwereade__: reviewed08:13
rogpeppefwereade__: afk for a little bit, back in a short while08:14
rogpeppefwereade__: back08:21
fwereade__rogpeppe, I think you're right about the Reset but I got distracted by broken trunk :/08:23
rogpeppefwereade__: that there should be a Flush in the TearDownTest too?08:25
fwereade__rogpeppe, yeah, but I need to think about that bit a little more08:25
rogpeppefwereade__: shall we have that chat?08:27
fwereade__rogpeppe, with you in a sec, would you start a hangout please?08:27
rogpeppefwereade__: https://plus.google.com/hangouts/_/405ae790bfb55499116d2961f7e7f2ecc352fb68?authuser=0&hl=en-GB08:28
rogpeppefwereade__: any chance you could jump back into that hangout? i just wanted to chat about one other thing08:56
fwereade__rogpeppe, sorry, totally didn't see that09:12
rogpeppefwereade__: np. i was just wonderin about the suggestion to drop version.Current.09:12
rogpeppefwereade__: it seems a bit odd to me - there whole point is so that a given binary can know what version it is09:13
fwereade__rogpeppe, I don't think it's sane, I'm responding now09:13
rogpeppefwereade__: and report it as such09:13
rogpeppefwereade__: ok, cool09:13
rogpeppedavechen1y: hiya09:14
rogpeppedavechen1y: can you tell us why dropping version.Current is a good idea?09:14
davechen1yrogpeppe: because it is non deterministic09:15
davechen1yi can build and test a cli on one series, and run it on another series and it does different things09:16
rogpeppedavechen1y: but version.Current isn't about testing09:16
rogpeppedavechen1y: it's about giving a unit the ability to find out what version it's running09:16
rogpeppedavechen1y: or a juju binary too09:17
davechen1yyes, and it is that multitude of roles that is causing the problem09:17
davechen1yi think version.Current needs to be replaced by several things which are specific to the role of the caller09:17
fwereade__davechen1y, many uses of version.Current are crack but many are not09:17
rogpeppedavechen1y: how would you find out the current version?09:17
davechen1yrogpeppe: sure, that is _1_ use for version.Current09:18
davechen1yarguably the correct one09:18
rogpeppedavechen1y: well, that's why i implemented it...09:18
davechen1ybut the use of version.Current for 'fuck, I need a version value, but I don't have one, fuckit, version.Current works'09:18
rogpeppedavechen1y: agreed totally09:18
davechen1ythroughout the code is causing tim problems09:18
rogpeppedavechen1y: but that doesn't imply that we should drop version.Current09:18
davechen1yand it is _those_ cases we need to fix09:18
rogpeppedavechen1y: just that we shouldn't use it in tests09:18
davechen1yrogpeppe: please don't read too much into my use of the word drop09:19
davechen1yby 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
davechen1ythen, once we've found and fixed them, we can put version.Current back09:19
davechen1ythis may probably happening within one change proposal09:19
rogpeppedavechen1y: 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
rogpeppedavechen1y: i don't think we need to remove it09:20
rogpeppedavechen1y: just grep for it09:20
davechen1ywell, tim wants to kill it, and I want to bisect it09:20
rogpeppedavechen1y: tim has no proposal for how you would *actually* find the current version09:20
davechen1yrogpeppe: i cna't speak for him fully09:21
davechen1ybut I believe he believes most of those pieces of data come from the environment configuration09:21
rogpeppedavechen1y: another possibility is to always set version.Current at the start of the tests, to a known value09:21
davechen1yi cannot speak further to this09:21
rogpeppedavechen1y: actually none of those pieces of data come directly from the env config09:21
davechen1yrogpeppe: like I said, I cannot explain his plans09:22
davechen1yi am not fully aware of them09:22
rogpeppedavechen1y: np. sorry, i thought you'd discussed it at length.09:22
davechen1ythis is only what I recally from our conversations09:22
davechen1ywe'd talked at length about the problme09:22
davechen1y... less so about the solution09:22
rogpeppedavechen1y: what do you think about just setting version.Current to a known value at the start of tests?09:23
davechen1yi don't think tim will accept that09:24
davechen1yand that doesn't address the problem that we use it in too many places for too many things09:24
rogpeppedavechen1y: then testing.DefaultVersion() ?09:25
davechen1yrogpeppe: i don't want to speak too much too this09:25
davechen1yi don't know tims plans09:25
rogpeppedavechen1y: ok09:25
davechen1ybut this is more that just testing09:25
davechen1yand is related to the set of cards he has to do with inserting series into start instance09:25
rogpeppedavechen1y: ok09:26
rogpeppedavechen1y: thanks for the input09:27
davechen1yrogpeppe: related, your issue you raised that looks like it was a crash in the hash map code09:27
davechen1ythere are two things that might be causing that09:27
davechen1y1, is gc can run concurrently with hashmap insertion09:27
davechen1yand corrupt the hash map09:27
davechen1ywe believe we've fixed those bugs now09:27
davechen1yso tip right now shouldn't do that09:28
davechen1yassuming that is the cause09:28
davechen1y2. this might actually be reall concurrent hash map mutation09:28
rogpeppedavechen1y: it died in append - i didn't think it looked like it was in the hashmap code09:28
rogpeppedavechen1y: but perhaps you were looking at some other aspect of the trace09:28
davechen1ybut the gdb you got pointed to the hashmap interator09:28
davechen1yfor sure, things with the new GC are not all beer and skittles09:28
rogpeppedavechen1y: ah, the second one, yes09:29
rogpeppedavechen1y: yeah, i'm not really surprised - it's a big change09:29
rogpeppedavechen1y: i don't know that i entirely trust the disass output though - i was dumping a binary that might have been different from the original09:30
davechen1yoh09:30
davechen1ywell that could point the finger at anything09:30
rogpeppedavechen1y: well, i *think* i rebuilt the same binary, but i'm not entirely sure09:31
* davechen1y wishes he'd been able to convince rsc to allow go test -c ./...09:31
rogpeppedavechen1y: i really tried (perhaps a bit too hard actually)09:31
rogpeppedavechen1y: workaround: http://paste.ubuntu.com/5636443/ (i call it "gotest-c")09:32
davechen1ynah, rsc was right, we're trying to get 1.1 out the door09:32
davechen1ynot the time for new features09:32
davechen1yand we can always just patch our go trees09:32
davechen1yor use your workaround09:32
rogpeppedavechen1y: true 'nuff09:33
rogpeppedavechen1y: 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 boundary09:33
davechen1yrogpeppe: good point09:38
davechen1yi was tlaking to jam on wed that cross compiling for win32 would be much simpler if we didn't use cgo for goyaml09:38
rogpeppedavechen1y: if only we could just ditch yaml09:42
TheMuerogpeppe: +1 ;)09:42
davechen1yrogpeppe: i like the cut of your jib, sir09:43
davechen1yfwereade__: could I trouble you to finish the sentence you started yesterday, https://docs.google.com/a/canonical.com/document/d/1d_b6VTt1bNDCNCrrIVEmEtNIoZi1toGvt5MvVBnxGxw/edit09:43
fwereade__davechen1y, done, sorry09:45
davechen1yfwereade__: no problem09:46
davechen1yif the tree passes i'll do the release tomorrow morning09:46
fwereade__davechen1y, cool, thanks09:47
TheMueso, with the latest change all test should pass (just running). then final cleanup and propose.10:13
davechen1yTheMue: yes, the tree passes at the moment10:13
davechen1ydid anyone hear from jam or mgz about a public bucket to push tools' for hp cloud ?10:14
TheMue*hmpf* one fail left10:21
davechen1yTheMue: my tree passed, what is failing for you ?10:27
TheMuedavechen1y: not the current trunk, only my branch with my changes. ;) they affect several places.10:38
davechen1ysadface10:40
TheMuedavechen1y: why?10:42
fwereade__TheMue, things affecting several places :)10:42
fwereade__davechen1y, sorry, I don't know about an hp public bucket10:43
TheMuefwereade__: hehe, yep, always not simple.10:43
davechen1ys'ok10:43
davechen1yi wrote to jam and mgz10:43
davechen1yworse case we can always publish an adendum to the release notes10:43
fwereade__davechen1y, sgtm10:43
TheMue*ouch* that panic has been new and definitely not caused by a change. will repeat the test, maybe an intermittent one.10:44
TheMueyip, passes10:46
fwereade__TheMue, if so, please make sure it's recorded10:46
TheMuefwereade__: yep10:46
TheMue*very-big-small* all pass10:56
TheMues/small/smile/10:57
TheMuelunchtime11:35
bachi rogpeppe12:54
rogpeppebac: hiya12:54
bacrogpeppe: i've got a question about charm Meta -- you the right guy?12:55
rogpeppebac: ... maybe :-)12:55
bacrogpeppe: 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 rule12:56
bacso 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 112:57
dimiternbac: take a look at charm/meta.go how the yaml gets parsed and default values filled in12:57
bacdimitern: i have and i didn't find my answer.12:57
rogpeppebac: hmm, i wasn't previously aware of the "limit" attribute12:58
bacdimitern: i don't, however, understand the 'coerce' bits12:58
rogpeppebac: Coerce is kinda clever12:59
rogpeppefwereade__: does limit limit the number of units that can join a particular relation?13:00
dimiternbac: judging from Coerce and the schema (ifaceSchema) limit can be either nil (empty, like "limit: ") or an int13:00
fwereade__rogpeppe, I *think* that limit is meant to mean how many services that relation can be established with13:01
rogpeppefwereade__: i think that's what i meant actually13:01
fwereade__rogpeppe, but to the best of my knowledge there has never been code that actually interprets it13:01
rogpeppefwereade__: ok, so bac's question is kinda moot13:01
fwereade__rogpeppe, yeah13:02
fwereade__rogpeppe, there's a bug for it somewhere13:02
fwereade__rogpeppe, and also for, er, is it "required"?13:02
dimiternfwereade__: yeah, that was weird for me when i saw it as well13:02
fwereade__rogpeppe, some other meta relation field anyway13:02
rogpeppefwereade__: i'm trying to think of a scenario where it might be useful13:03
bacrogpeppe: 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
rogpeppebac: ha, i see.13:03
fwereade__rogpeppe, it doesn't make much sense to have wordpress talking to two different databases13:03
bacif that is the case, i'd rather just remove it from the results i return.13:03
fwereade__bac, +1 to not representing it in the gui13:03
rogpeppefwereade__: hmm, i didn't realise you could have a "requires" relation with more than one service13:04
bacfwereade__: ok, so remove it from the results i send back.  sound good to me.13:04
fwereade__bac, sgtm13:04
rogpeppefwereade__: 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:05
fwereade__rogpeppe, I am not aware of anything that would prevent it13:06
dimiternmramm: 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
rogpeppefwereade__: hmm13:06
fwereade__dimitern, I think we decided to go with the simplest thing for now13:06
dimiternfwereade__: ah, ok then13:07
fwereade__rogpeppe, TheMue, mramm: I have to pick up laura today, I won't make the kanban meeting I'm afraid13:08
rogpeppefwereade__: ok13:08
TheMuefwereade__: ok, thx for the info.13:08
=== wedgwood_away is now known as wedgwood
=== bcsaller_ is now known as bcsaller
mrammdimitern: 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
mrammand I thought we had decided to just do 1 person for now13:54
mrammbut if we need time-zone spread, we can do that13:54
dimiternmramm: well, we can have 1 guy around AU and 1 around EU each day maybe?13:54
mrammwell, that means that the AU guys have to do it more than 1x week13:55
dimiternmramm: for now it's ok, but we should think about it if it proves insufficient (slow reviews e.g.)13:55
mrammright13:55
mrammwe will improve as needed13:55
=== benji___ is now known as benji
bacrogpeppe, dimitern: if you've got time to do a review i'd appreciate it:  https://codereview.appspot.com/766804514:36
dimiternbac: i'll take a look in a bit14:36
bacthanks!14:36
rogpeppebac: me too14:37
bac\o/14:37
dimiternbac: reviewed14:54
bacthanks14:54
fwereade__TheMue, would you explain the high-level thinking behind the various funcs for manipulating home/juju_home?14:58
fwereade__TheMue, I'm having some difficulty forming a picture of what gets called when and why14:58
rogpeppefwereade__: latest in the version.Current saga: tests fail when run on quantal 38615:14
rogpeppefwereade__: because imagesData doesn't have any image for that combination15:15
fwereade__rogpeppe, I'm not surprised, I'm starting to think we basically shouldn't ever be using real series15:15
rogpeppefwereade__: in testing, yeah.15:15
fwereade__rogpeppe, indeed :)15:16
rogpeppefwereade__: except live tests, i guess15:16
fwereade__rogpeppe, true, but they need distinct config anyway15:16
rogpeppefwereade__: yup15:17
fwereade__TheMue, ping15:18
TheMuefwereade__: just fetched a tea ;) and wanted to answer your question15:19
fwereade__TheMue, heyhey, I was just looking at the JUJU_HOME stuff15:19
TheMuefwereade__: Init() is to init the juju home based on $JUJU_HOME or $HOME15:19
TheMuefwereade__: JujuHome() returns it15:20
fwereade__TheMue, the big question in my mind is why we're calling Init() in test setup15:20
TheMueJujuHomePath() allows to build a path to a file/dir in juju home15:20
TheMuefwereade__: because otherwise some tests are failing (with a panic) when they retrieve the juju home somewhere in a nested function15:21
TheMuefwereade__: I first tried it w/o any init and then digged down step by step15:21
fwereade__TheMue, but why would we want to require that HOME or JUJU_HOME be set in order to run tests?15:22
TheMuefwereade__: so if a test tests Foo(), which calls Bar() and that Yadda() and Yadda() references to the juju home, you simply need it15:22
fwereade__TheMue, no15:22
fwereade__TheMue, we should not need any magic env vars set just to run the tests15:22
TheMuefwereade__: we do, already now15:23
fwereade__TheMue, those? or others?15:23
TheMuefwereade__: we have many code, which relies on $HOME15:23
TheMuefwereade__: and those tests would fail w/o a $HOME too15:23
fwereade__TheMue, I thought this branch was replacing that?15:23
TheMuefwereade__: this branch now replaces the static init()15:24
TheMuefwereade__: but the code using $HOME is nested15:25
TheMuefwereade__: we said JujuHome() shall panic, if $HOME or $JUJU_HOME are not set15:25
fwereade__TheMue, so what? surely the tests should be manipulating exactly one thing -- the value of jujuHome15:25
TheMuefwereade__: so the replacement of the old direct code with the usage of the new API would panic15:26
fwereade__TheMue, and we have SetTestJujuHome for that15:26
fwereade__TheMue, but you have made STJH panic if Init hasn't been called, which STM to be missing the point15:27
TheMuefwereade__: ok, sounds reasonable15:27
fwereade__TheMue, also, what's the deal with origJujuHome?15:27
TheMuefwereade__: 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
fwereade__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
TheMuefwereade__: it allows to restore it, even if $JUJU_HOME as env var has been changed inside the tests15:28
fwereade__TheMue, well, kinda15:29
TheMuefwereade__: one moment, cleaned up my editor, have to find the place15:29
fwereade__TheMue, but it stops you being able to nest `defer STJH(STJH(c.MkDir()))`15:29
fwereade__TheMue, if you do that when it's already been done somewhere else, things will act weird15:30
TheMuefwereade__: https://codereview.appspot.com/7923043/diff/12001/environs/cloudinit/cloudinit_test.go?column_width=120 line 26ff15:31
fwereade__TheMue, I see no reason to call Init there15:32
TheMuefwereade__: why do you want to nest setting a test home?15:32
fwereade__TheMue, ah, ok, I do -- Init and SetTestJujuHome are equally bad there15:32
TheMuefwereade__: ok, setting home should work too, yes. inside of config.New() today $HOME/.juju is accessed15:33
fwereade__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 over15:33
TheMuefwereade__: sorry, can't follow your last sentence15:34
fwereade__TheMue, sorry15:34
fwereade__TheMue, I'm saying that there is too much surprising action at a distance for me to follow15:35
TheMuefwereade__: but yes, I can follow your idea of using STJH() also for an initializing in tests. here I thought too short15:35
fwereade__TheMue, but I don't seem to have communicated why it is wrong to call Init in tests?15:36
fwereade__TheMue, I wouldn't have STJH in code but I'm not typing it out every time in irc ;)15:36
fwereade__TheMue, you should not be calling Init in tests because it's making a global state change that cannot be restored cannot be restored15:37
TheMuefwereade__: what I meant is, that STJH shouldn't rely on an Init before15:37
fwereade__TheMue, ah, got you15:38
TheMuefwereade__: it can be restored15:38
TheMuefwereade__: that's why ...Orig is only set once and never changed again15:38
fwereade__TheMue, original correct state -- with jujuHome="" -- *cannot* be restored15:39
fwereade__TheMue, it will panic if you try15:39
fwereade__TheMue, if you put in Init in one test, other tests in the package might or might not run Inited, dependingon circumstance15:39
fwereade__TheMue, am I making sense to you?15:40
TheMuefwereade__: would have like to see it painted on a whiteboard. *lol*15:41
TheMuefwereade__: but yes, Restore... relies on Init before15:41
TheMuefwereade__: and that's the failure15:41
fwereade__TheMue, what's the actual use case for Restore... anyway?15:42
fwereade__TheMue, can't we just just STJH throughout?15:42
TheMuefwereade__: hmm, would be possible. but I would like a mechanism like in FakeHome more. an own type with a Restore method15:43
TheMuefwereade__: so the call would be defer STJH("foo").Restore()15:43
fwereade__TheMue, that would be fine too, my problem is that we have a load of different ways to do basically the same thing15:43
TheMuefwereade__: yes, we have, the problem with a universal programming language ;) but rogers idea is very elegant15:44
fwereade__TheMue, in fact that would be nice, because you can just stick it in a field with clear intent15:45
fwereade__TheMue, sorry, which idea?15:45
TheMuefwereade__: the idea of an own type (based on a string) with a Restore method15:45
fwereade__TheMue, we have two fakeHome types already I think, I would be hoping to merge those rather than to create a new one:)15:46
TheMuefwereade__: 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
fwereade__TheMue, but the main point is that config.Init() must not be called in tests, except for those testing itself15:49
TheMuefwereade__: yep, got you15:49
fwereade__TheMue, long review sent I'm afraid16:10
TheMuefwereade__: ok, thx. will handle it with care. ;)16:10
fwereade__aww poo, self-evaluation16:12
fwereade__guess I won't be getting anything else done today16:12
TheMuefwereade__: btw, the cachePath change to $JUJU_HOME/cache works because of the setting of $JUJU_HOME in Init if it not yet set16:15
rogpeppefwereade__: oh bugger, that16:15
* TheMue done this evaluation stuff this morning16:15
fwereade__TheMue, but that doesn't happen16:15
TheMuefwereade__: what doesn't happen?16:16
fwereade__TheMue, setting $JUJU_HOME in Init16:16
fwereade__TheMue, which is a good thing, because I can think of no good reason to do so16:17
fwereade__TheMue, but it does mean the charm store will only work in very specialized circumstances16:17
rogpeppefwereade__: 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/iyfOvjcWR116:17
rogpeppefwereade__: answer: it would be possible, but quite expensive.16:17
TheMuefwereade__: argh, sorry, missed it when changing the code. had it in there before.16:17
fwereade__rogpeppe, that's cool, but, yeah, not lightweight16:18
rogpeppefwereade__: if our ops on mongo weren't so slow, it might be feasible16:18
rogpeppefwereade__: the nice thing is it can test arbitrary transactions together in a thorough way16:19
fwereade__rogpeppe, that is very nice, indeed16:20
rogpeppefwereade__: 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
fwereade__rogpeppe, yeah, I would be very keen on future work in this direction16:21
rogpeppefwereade__: i'll save it for later :-)16:22
rogpeppefwereade__: the only major assumption is that when an operation generates a transaction, it's a deterministic function of the state it's previously seen.16:27
=== deryck is now known as deryck[lunch]
fwereade__happy weekends everyone17:44
dimiternfwereade__: you too! :)17:44
rogpeppefwereade__: any you!17:52
rogpeppes/any/and17:52
=== deryck[lunch] is now known as deryck
rogpeppehappy weekends all!18:48
dimiternrogpeppe: same to you :)18:53
=== wedgwood is now known as wedgwood_away

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!