bigjoolsthumper: is this likely to be fallout from your series changes? http://pastebin.ubuntu.com/5691125/02:28
bigjoolsI am trying to work out if it's a bug in the core or in the maas provider02:29
* thumper looks02:29
thumperbigjools: yes, you want --fake-series=precise02:30
bigjoolsthumper: on the bootstrap command line?02:30
bigjoolsok cheers02:30
thumperas you are uploading tools02:30
thumperand by default, it makes tools for your series02:30
thumperbut the machine you are booting says precise02:30
bigjoolsit worked \o/02:31
thumperalthough you should have public tools for 1.9.13 now02:31
bigjoolsthumper: public?02:32
thumperas in, ec2 public bucket02:32
bigjoolsthumper: I am not using ec202:32
bigjoolsthis is maas02:32
* thumper nods, ok02:32
bigjoolsanyway, bootstrap "completed" so thanks :)02:33
bigjoolsis there a local placement option in juju-core like there was in Python?04:36
markrammthere is one under review right now04:36
bigjoolsok ta04:37
markrammbigjools: it will function similarly to deploy-to but be called force-machine04:37
markrammdiscussion on the mailing list about it is ongoing04:37
bigjoolsmarkramm: I don't know much about this - is deploy-to a cmd line option?04:37
davecheneybigjools: no, that isn't an option at the moment04:39
markrammbigjools: it was a jitsu feature in python04:41
markrammthat took a machine ID and installed a service on that particular machine04:41
bigjoolsah ok, no wonder I hadn't heard of it04:42
bigjoolsthanks both04:42
markrammI just now realize that I'm not sure what you meant by "local placement"04:42
markrammso perhaps I got it wrong04:42
bigjoolsmarkramm: "placement: local" in the environment config04:43
bigjoolsjuju-core complains if I have that in04:43
davecheneybigjools: yes, we don't understand that key04:45
bigjoolsdavecheney: the error message was a little opaque: error: placement: expected nothing, got "local"04:46
davecheneyyeah, that isn't that helpful04:46
* bigjools files a bug04:46
davecheneybigjools: good idea04:46
bigjoolsI was actually looking to avoid bringing up another machine in maas and re-using the bootstrap node04:46
bigjoolssince it's slow - there's no fast installer yet04:47
bigjoolsbut no biggie04:47
davecheneybigjools: https://launchpad.net/ubuntu/raring/+source/mongodb/1:2.2.4-0ubuntu105:20
davecheneyis it possible to drag this into a PPA targeting precise ?05:21
bigjoolsit might be, depends if it builds with precise's available deps05:21
davecheneyshould I try a source build on a Precise vm05:28
* thumper goes to make dinner05:42
bigjoolsdavecheney: use pbuilder05:43
davecheneybigjools: speak to me as if I were a child05:43
bigjoolswell you can use a VM05:44
bigjoolsdavecheney: I've got 4 naughty kids, don't tempt me!05:44
davecheneybigjools: if you want to come to NSW for a spankin' let me know05:44
davecheneyi'm happy to arrange05:44
bigjoolswhat, you'd whizz straight past me then05:45
bigjoolsso you can use a VM, or pbuilder automates an environment for a particular release05:45
bigjoolswhatever you're comfortable with05:45
davecheneyi'll try pbuilder05:46
davecheneyi expect to get screwed on permissings05:46
bigjoolsfrankly, I fire up a canonistack instance these days05:46
bigjoolsmassively quicker05:47
* davecheney deployes the ubuntu charm05:47
bigjoolsjust dget the source for raring on a precise instance05:47
* bigjools afk05:50
rogpeppemornin' all!06:33
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: TheMue | Bugs: 0 Critical, 52 High - https://bugs.launchpad.net/juju-core/
TheMuemoin moin btw07:31
rogpeppeTheMue: hiya08:11
rogpeppeTheMue: i believe you are the on-call reviewer today...08:11
rogpeppeTheMue: i have some reviews for you :-)08:11
rogpeppeTheMue: https://codereview.appspot.com/8458044/ https://codereview.appspot.com/8487044/ https://codereview.appspot.com/8533044/ https://codereview.appspot.com/8534043/08:11
rogpeppefwereade: if you fancy having a look, another opinion would be nice too08:12
fwereaderogpeppe, cheers, I'll try to get there soon08:12
rogpeppefwereade: thanks08:12
rogpeppefwereade: that last one gets us back to having unit and machine status for the gui folks08:13
TheMuerogpeppe: will look at them.08:20
rogpeppeTheMue: thanks!08:20
TheMuerogpeppe: a lot of stuff, but i'm getting forward09:24
rogpeppeTheMue: thanks a lot. the big changes are pretty much mechanical09:25
TheMuerogpeppe: yes, that helps09:26
rogpeppeTheMue: thanks for the review. i'm not sure what you're thinking of when you say "exposed constants for the EntityTypes"09:41
TheMuerogpeppe: simple public constants, nothing more. but as i also wrote, today they are needed nowhere, so simply forget it. ;)09:45
rogpeppeTheMue: sorry, do you mean the entity kinds? i don't see any occurrence of the word EntityType in the source.09:46
TheMuerogpeppe: oh, sorry, yes. where you return strings like "service" etc.09:47
rogpeppeTheMue: ah, i see.09:47
rogpeppeTheMue: they are actually compared, but not outside the params package.09:48
TheMuerogpeppe: i haven't seen them as strings elsewhere, only that you fill that one map by reflection. am i right?09:49
rogpeppeTheMue: look for the string "machine" in state/api/params and you'll see what i mean09:50
danilosheya, I am getting a bunch of test failures with juju-core trunk on raring: http://pastebin.ubuntu.com/5691888/09:51
TheMuerogpeppe: ah, ic. so probably constants make sense, but not public ones.09:51
danilosI did follow README and CONTRIBUTING docs, except that I am using system-installed mongodb09:52
rogpeppeTheMue: i'm happy just using the strings. test coverage is sufficient that you won't be able to get it wrong.09:52
danilos(because it's 2.2.4 and 2.2.0 was referenced in the README)09:52
danilosanyone has any ideas what did I mess up?09:52
rogpeppedanilos: i think you do need the SSL-capable mongo09:52
rogpeppedanilos: it looks like you might be using GOPATH wrongly09:53
rogpeppedanilos: what's the value of your $GOPATH ?09:53
danilosrogpeppe, it's /home/danilo/.gopkgs09:53
danilosrogpeppe, I've got a symlink from ~/.gopkgs/src/launchpad.net/juju-core to ~/juju-core, but it behaves the same no matter what directory I am in09:54
danilosrogpeppe, or maybe not the same, but I've seen a lot of test failures either way09:54
rogpeppedanilos: symlinks don't work, i'm afraid09:55
rogpeppedanilos: your juju source must live in /home/danilo/.gopkgs/src/launchpad.net/juju-core09:55
rogpeppedanilos: you can symlink the other way if you want09:55
danilosrogpeppe, :/ but originally I tried it like that as well, maybe the non-SSL mongo was to blame09:55
rogpeppedanilos: you definitely need the SSL-capable mongo09:56
rogpeppedanilos: were you getting build errors before?09:56
danilosrogpeppe, no, go build seems to work correctly either way09:57
rogpeppedanilos: sorry, i'm mean build errors when testing09:57
rogpeppedanilos: like the errors you pasted above09:57
rogpeppedanilos: the easiest way to start is to do: "go get launchpad.net/juju-core/..."09:58
rogpeppedanilos: making sure you remove that symlink first09:58
danilosrogpeppe, right, that's how I started off then rearranged the trees a bit to follow my branch organization model (which is a shared repo with lightweight checkouts: I like having multiple branches around, especially when starting on a project)09:58
rogpeppedanilos: that will get you the source into the right place09:58
mgzgo is really picky about placement of things09:59
rogpeppedanilos: i use cobzr09:59
rogpeppedanilos: fwereade doesn't but i don't know how he *does* use multiple branches09:59
danilosrogpeppe, right, I hate the git model of a single working directory and local merge problems when you forget to commit stuff, which is why I tried to avoid it09:59
mgzjam has a setup like yours, I'll just see if I have what his layout was anywhere09:59
mgzit certainly can be made to work09:59
mgz...I think thumper does as well10:00
rogpeppemgz: i think thumper uses native bzr colocation10:00
* mgz has been using this to test the 2.6 colo stuff10:00
danilosrogpeppe, it seems there are no build failures when I get the code directly in, argh :/ there're still a bunch of test failures10:01
rogpeppedanilos: are you using the right version of mongo?10:01
danilosmgz, rogpeppe: ok, I'll check with them if I don't figure something out, and switch to cobzr model until then10:02
danilosrogpeppe, not yet, I'll get it downloaded and set up as well10:02
rogpeppedanilos: all you need to do is download the tar file and put the files somewhere in your $PATH10:02
danilosrogpeppe, ok10:04
danilosrogpeppe, so far it seems that was the remaining problem; I'll figure out a tree layout that works for me, thanks for the input10:08
rogpeppedanilos: cool, np10:08
mgzI know one thing that works, is having the normal shared-repo+treeless branches in your normal dev location10:09
mgzthen lightweight checkouts in the go-expected locations10:09
mgzthere's a trick then to make switch easy, by doing something in locations.conf I think, but am not sure what it is10:10
danilosmgz, right, and can probably live with something like that, thanks10:15
rogpeppefwereade: i'd really appreciate your comments on this branch before it gets waved through, as it's the technique that's directly impacted by the doc split between unit/machine and status. https://codereview.appspot.com/8534043/10:19
fwereaderogpeppe, ok, looking now10:20
rogpeppefwereade: thanks10:22
fwereaderogpeppe, that one's approved10:31
fwereaderogpeppe, need to grab some lunch, bbiab10:32
rogpeppefwereade: thanks!10:32
rogpeppefwereade: ping10:50
fwereaderogpeppe, pong11:08
rogpeppefwereade: i was just wondering why we don't just use constraints.Value directly as a mongo doc11:09
dimiternmgz: standup?11:30
mgzI'm there11:31
danilosmgz, dimitern: https://pastebin.linaro.org/2129/11:32
danilosmgz, dimitern: http://pastebin.ubuntu.com/5692068/11:33
dimiternTheMue: cheers for updating the topic!11:37
TheMuedimitern: yep, seen it as my task as reviewer today. ;)11:38
fwereaderogpeppe, sorry, I left a comment half-types11:38
fwereaderogpeppe, because I'd rather have a dumb but explicit translation layer between the api type and the database representation than to conflate the two11:39
dimiternno critical bugs? how come?11:39
rogpeppefwereade: do you think we should do that for all the other types we marshal directly to mongo then?11:39
rogpeppefwereade: i'm wondering if we should marshal constraints to mongo as string11:40
bacanyone have time for a quick review? https://codereview.appspot.com/853204311:40
fwereaderogpeppe, I can't think of many of those... state.Tools, any others?11:40
rogpeppefwereade: charm.Meta11:40
rogpeppefwereade: (that's a big one)11:41
fwereaderogpeppe, hmm, yeah, in hindsight the pressures maybe feel different now constraints is out of state11:42
rogpeppefwereade: charm.Config11:42
rogpeppefwereade: i'd quite like a tool that automatically checked for mongo compatibility between versions11:43
rogpeppefwereade: but i'm not entirely sure how it would work11:43
fwereaderogpeppe, yeah, I think it'd be tricky11:44
rogpeppefwereade: there are two approaches: 1) make some kind of formal description of the schema and check that everything adheres to it 2) write a test that talks to two different versions, each of which looks at the same mongodb, getting one to write and the other to read11:46
rogpeppefwereade: (at least) two approaches :-)11:46
fwereaderogpeppe, yeah, it's not going to be fun11:47
rogpeppefwereade: maintaining compatibility is crucial though.11:48
fwereaderogpeppe, agreed11:48
rogpeppefwereade: there's another possibility actually11:48
fwereaderogpeppe, I'm honestly not sure about external types, but I am starting to feel that from this perspective it's important that we keep a clear eye on just wtf we are storing in mongo11:49
rogpeppefwereade: use a tool to tell us exactly what types we're storing in mongo11:49
rogpeppefwereade: then we can use a variant of the go compatibility checker to make sure we don't change those types in backwardly incompatible ways.11:49
rogpeppefwereade: it won't work for types that know how to marshal themselves, but those are probably not where the problems will occur11:50
rogpeppefwereade: because if you change a MarshalBSON method, you'll have a pretty good idea that it might have some impact on mongo storage11:51
fwereaderogpeppe, well, it's the types that know how to marshal themselves I am concerned about tbh -- those changes are going to be easy to make accidentally11:51
rogpeppefwereade: i'm more concerned about someone just changing the type of a field11:51
rogpeppefwereade: without realising that the type is used in mongo11:52
fwereaderogpeppe, yep, that too11:52
fwereaderogpeppe, agreed that having what you described in place would take one big worry off the table11:52
fwereaderogpeppe, in the meantime I think I *would* prefer to avoid directly storing external types in mongo, but obviously we can't change all that now, we just need to fret and hope :/11:53
rogpeppefwereade: for now, perhaps we should just comment every type that's used in mongo, so that people know it's important not to change it.11:54
rogpeppefwereade: it should be straightforward to do11:54
fwereaderogpeppe, that sounds like a good idea to me11:54
rogpeppefwereade: i've added a ticket to the kanban11:59
fwereaderogpeppe, cheers11:59
danilosmgz, dimitern: the other test seems to really be fixed by r39 in goyaml from a week or so ago by gustavo12:11
dimiternrogpeppe, fwereade: next chapter in the nonced provisioning saga: https://codereview.appspot.com/8561044/12:14
mgztrivial test fix branch to review: https://codereview.appspot.com/857004312:15
mgzdanilos: ^you can practice your reitveld on that too :)12:15
danilosmgz, I will, that's even easier :)12:16
danilosI am having some trouble getting lbox to do anything with codereview.appspot.com: I suppose I need to configure something (even if I set my email address for bzr to the one I use to log in to codereview.appspot.com, no review is created [one is created on LP])12:49
rogpeppeTheMue: here's a review for you: https://codereview.appspot.com/856804413:22
danilosah, ok, so if updating the milestone fails (which I don't have permissions for yet), then it borks out and doesn't do a code review submit13:22
TheMuerogpeppe: just doing dimiters, then yours will follow13:22
rogpeppeTheMue: ta!13:23
dimiterndanilos: how did you manage to update a milestone with lbox? :)13:23
danilosdimitern, I didn't, but it tries to update a bug milestone and I am still not a member of the ~juju team which is the maintainer for the juju-core project13:23
danilosdimitern, I am only passing the -bug=... parameter to the propose command13:24
dimiterndanilos: I see13:24
dimiterndanilos: didn't know it touches milestones when you link a bug13:24
danilosdimitern, yeah, it tries to create a bug if you don't link it (or so it seems)13:25
danilosnow I need Kapil, Gustavo or Dave Cheney to add me to the team13:26
fwereadedimitern, reviewed13:26
daniloshazmat, hi, can you please add me to the ~juju team?13:26
dimiternfwereade: cheers!13:27
dimiternfwereade:  and this is the final step: https://codereview.appspot.com/8561045/13:28
fwereadedimitern, ta13:28
hazmatdanilos, welcome to juju core, and done13:28
daniloshazmat, thanks! :)13:30
dimiternfwereade: I don't think asserting on both instanceid and nonce being "" will work for replayed transactions13:31
danilosTheMue, hi, I wouldn't mind getting a review for https://codereview.appspot.com/8572043/ :)13:32
fwereadedimitern, I think replayed transactions leak badly here anyway13:33
fwereadedimitern, but in the actual use cases, we'll never be setting the same instance id anyway, so that specific check actually reduces to "never set" anyway13:33
TheMuedanilos: enqueued, just doing rogers ;)13:33
danilosTheMue, thanks13:33
danilosmgz, dimitern: I wouldn't mind if one of you gives me a second ack (or nack, depending on your opinion :) on https://codereview.appspot.com/8572043/13:34
fwereadedimitern, I think it's clearer and simpler and no less correct ;p13:34
dimiternfwereade: it's simpler for sure, just have a nagging feeling I'm missing something, if i don't assert instid should either be "" or "instid"13:36
dimiternfwereade: perhaps you could alleviate my concern - what'll happen when we need to replay a SetProvisioned transaction without asserting "instid", which is already set?13:37
fwereadedimitern, I think there is a general issue wrt txns that need to be synchronized with non-txn state13:37
fwereadedimitern, the short answer is "we're probably screwed", but asserting on the possibility of the same instance id can't actually help, I think13:37
dimiternfwereade: istm asserting only on "" will cause erraborted, which then by the virtue of the machine being still alive will return "already set"13:38
dimiternfwereade: that's the only reason to check for same id in the assert, if you think it's superfluous, I'll drop it13:39
dimiterndanilos: you've got a review13:41
fwereadedimitern, essentially a delayed SetProvisioned txn leaves state badly inconsistent with reality until it's completed, and there's no way round that except to do some distracting cleverness re completing machine txns before making provisioning-related decisions13:41
fwereadedimitern, and it's low-impact enough that I plan to just skip it for now13:42
dimiternfwereade: yeah, and hard to induce such an error to test it properly imo13:43
dimiternfwereade: ok, I'll do as you suggest13:43
fwereadedimitern, yeah, indeed13:43
danilosdimitern, thanks13:47
danilosdimitern, fwiw, I did test it with both tip revisions and going back one revision of goyaml to confirm it fails expectedly with it13:48
dimiterndanilos: cheers13:54
TheMuerogpeppe, danilos: you've got reviews13:55
rogpeppeTheMue: ta!13:55
TheMuerogpeppe: yw, nice little helpers in the code13:56
TheMuedanilos: does go get -u retrieves the fixed goyaml version?13:58
danilosTheMue, I can try reverting to r38 to see if it does (since I started yesterday, I got the latest version by default: r39 is from April 2nd)13:58
danilosTheMue, yeah, it does update it for me at least14:00
TheMuedanilos: that's good14:01
danilosTheMue, btw, is "LGTM" a convention of rietveld (i.e. does it interpret it as a positive review and makes the review message green), or just internal juju team convention to give out a shorter "looks good to me"?14:01
mgzit's for the pretty green14:01
danilosmgz, for some definition of pretty, yes :)14:02
* TheMue almost missed hangout14:02
TheMuedanilos: it is interpreted by rietveld, and "not lgtm" marks it red14:03
dimiterndanilos: and there's the red NOT LGTM for Disapprove14:03
danilosTheMue, dimitern: ack, thanks14:03
dimiterndanilos: not sure if it's case-sensitive, but it definitely catches it as part of the word (like "this is LGTMy"), ymmv14:05
dimiternfwereade: updated https://codereview.appspot.com/8561044/14:07
dimiternfwereade: and there's the other one as well - https://codereview.appspot.com/8561045/14:08
* dimitern lunch14:09
mgzheh, even later lunch than me14:10
danilosniemeyer, heya, I've marked a couple of bugs that you've fixed in goyaml with r39 as fix-committed, I hope you don't mind :)14:12
niemeyerdanilos: Thanks!14:36
niemeyerdanilos: They're even released already14:36
danilosniemeyer, even better :)14:39
rogpeppedanilos: are you Данило Шеган  ?14:46
rogpeppedanilos: 'cos if so, i think you've just earned your first "broken trunk" badge :-)14:47
TheMuerogpeppe: with an updated goyaml?14:48
rogpeppeTheMue: yes14:48
TheMuerogpeppe: oh, where does it break?14:49
rogpeppeTheMue: constraints14:50
TheMuedanilos: you've done go build ./... and go test ./... in the juju-core directory?14:50
danilosTheMue, yeah, with latest trunk14:50
danilosTheMue, are you seeing any problems?14:50
danilosrogpeppe, btw, I am, what did break for you?14:51
danilosrogpeppe, got the traceback?14:51
mgzha, that would be nice :)14:51
mgzcorrect term is "cryptic one-line error"14:52
rogpeppedanilos: http://paste.ubuntu.com/5692522/14:52
danilosrogpeppe, that's exactly what should fail if you don't have fixed goyaml version14:52
rogpeppemgz: aww come on, it's a cryptic four-line error, surely?!14:52
rogpeppedanilos: ah, so i need to pull goyaml?14:53
danilosrogpeppe, i.e. when I tested it with r38 of goyaml, it failed like that14:53
rogpeppedanilos: ah, i'm on v3614:53
danilosrogpeppe, yes, as per my email (this is goyaml r39 from 2013-04-02 by niemeyer): it was failing for me since I just fetched it yesterday for the first time14:53
mgzthat probably deserves a post to the mailing list then, if everyone needs to update14:54
mgz...which you've already done14:54
danilosmgz, I posted to juju-dev14:54
rogpeppedanilos: ahhhh, i think gustavo must've moved the goyaml repo14:54
rogpeppedanilos: because it thinks 36 is the latest version14:54
TheMuerogpeppe: that's why i asked for the goyaml. *phew*14:54
mgzyeah, need `bzr pull --remember lp:goyaml`14:55
danilosrogpeppe, what does 'bzr info' say for you in there?14:55
rogpeppedanilos: http://paste.ubuntu.com/5692535/14:55
mgzchanged from lp:~gophers/goymal/trunk14:55
danilosit should be something like bzr+ssh://bazaar.launchpad.net/+branch/goyaml off the top of my head, let me check on disk for me14:55
rogpeppeniemeyer: i've done "bzr pull" in my goyaml directory and it things 36 is the latest branch, but apparently there's a r39 out there14:56
mgzrogpeppe: ^see my bzr pull command above14:56
niemeyerIt's still lp:goyaml.. it's under ~goyaml's trunk now, indeed.. the change was announced when we switched everything out of ~gophers14:56
danilosrogpeppe, yeah, lp:goyaml points at http://bazaar.launchpad.net/~goyaml/goyaml/trunk/ for me (so not bzr+ssh, guess that's go thingy)14:56
niemeyerdanilos: Nope14:57
mgzyeah, go get is... not helpful14:57
niemeyerAh, yes14:57
rogpeppeniemeyer: ah, presumably i had to do a bzr pull --overwrite --remember lp:goyaml/trunk14:57
niemeyerrogpeppe: Hmm.. no..14:57
niemeyerrogpeppe: bzr pull lp:goyaml --remember14:57
danilosniemeyer, any other reason why it didn't save the "top" URL of lp:goyaml (which I think evaluates to bzr+ssh://bazaar.launchpad.net/+branch/goyaml or similar)14:58
niemeyerdanilos: I have no idea..14:58
rogpeppeniemeyer: ah, ok, done, thanks14:58
niemeyerdanilos: I'd expect that too14:58
* rogpeppe wonders what other packages are "frozen" in the same way14:59
danilosniemeyer, I guess it's not using "lp:" for fetching branches, which would explain it14:59
mgzyeah, jam proposed a fix for that14:59
niemeyerdanilos: Maybe.. I'd not expect that to make a difference, though14:59
niemeyerdanilos: Whatever is fetching branches is still talking about launchpad.net/goyaml14:59
mgzit may even have landed in go trunk15:00
niemeyerdanilos: Not ~foo/goyaml/trunk15:00
danilosniemeyer, maybe it's doing so over http (I had the http ~foo/goyaml/trunk in it after a simple 'go get', and my bzr login is set properly)15:01
niemeyerdanilos: Maybe, but I'd still consider that a bug in bzr itself15:01
niemeyerdanilos: If the user asks for lp.net/foo, that's what it should be linked with15:01
niemeyerdanilos: Even if it fetches from elsewhere15:02
niemeyerdanilos: We probably won't solve that bug today, though15:02
danilosniemeyer, yeah, or in Launchpad (I've just double-checked that, if I do a 'bzr pull --remember http://launchpad.net/goyaml', it does the wrong thing)15:02
dimiternstill waiting for a review on https://codereview.appspot.com/8561045/15:02
danilosniemeyer, indeed :)15:02
dimiternfwereade: sorry, but i'm bugging you again ^^15:02
mgzthere does seem to be a launchpad forwarding bug15:02
fwereadedimitern, sorry dude15:03
mgzbut... using that http url is... more than a little bogus anyway15:03
niemeyermgz: Hmm.. why?15:04
danilosmgz, yeah, LP sends permanent redirects for that http url (it's not perfect, but it seems to work and I suppose go get uses it)15:04
TheMuedimitern: started revisioning15:04
danilosniemeyer, I don't think it's advertised anywhere as the URL to use for fetching branches15:04
dimiternTheMue: thanks15:04
niemeyerdanilos: Well.. 1) It works; 2) It's extremely user-friendly15:04
danilosniemeyer, I am not disagreeing, I think there's a LP (or maybe bzr) bug in there15:06
mgzthe branch also has a broken parent reference15:06
rogpeppedanilos: i can't see your post to juju-dev anywhere. what was the subject line?15:06
niemeyerrogpeppe: Update goyaml dependency for juju-core15:07
rogpeppeniemeyer: ah, just turned up15:07
mgzniemeyer: can you run this please: `bzr config --scope=branch -d bzr+ssh://bazaar.launchpad.net/+branch/goyaml --remove parent_location`15:08
danilosrogpeppe, I've posted an update with your experience as well, hope this helps somebody else who hits the same problem15:08
rogpeppedanilos: thanks15:09
danilosrogpeppe, and now I see that mgz beat me to it... oh well :)15:09
niemeyermgz: What does that do?15:11
mgzremoves a broken parent location reference on the trunk branch15:12
mgzit's trunk, it shouldn't have a parent (and certainly not a ../../ one that goes nowhere)15:12
TheMuedimitern: you've got a review15:12
dimiternTheMue: tyvm15:12
niemeyermgz: Never seen that15:12
niemeyermgz: Neat15:13
niemeyermgz: The command, Imean15:13
mgzbeats editing .bzr/branch/branch.conf, especially when it's a remote branch :)15:14
* danilos hands the first "broken trunk" badge back ;)15:15
mgzokay, I think that probably does fix the launchpad redirect too15:16
mgzso, go get should now have the right origin15:16
niemeyermgz: Done15:16
danilosmgz, it does? I just filed https://bugs.launchpad.net/launchpad/+bug/1166854, let me check that :)15:23
_mup_Bug #1166854: bzr http URL stores evaluated URL instead of the link <Launchpad itself:New> < https://launchpad.net/bugs/1166854 >15:23
danilosmgz, it doesn't for me, it still stores the evaluated URL instead of the shorter one15:23
mgzoh, that bxr resolves redirects and stores the dest is a known thing15:24
danilosmgz, at least 'bzr pull --remember http://launchpad.net/goyaml' does (haven't tried 'go get' itself)15:24
mgzbut launchpad actually had the wrong redirect15:24
danilosmgz, right; oh, the redirect worked fine for me at least15:24
danilosmgz, btw, is bzr supposed to store temporary redirects as well? could that be a solution?15:25
danilosmgz, solution on the LP side, that is15:25
mgzthe launchpad side for this is very messy15:25
mgzthere are two levels, the first of which isn't actually an http redirect at all, but a fake branch that references another location15:25
danilosmgz, it's starting to sound nice15:26
danilosmgz, "interesting" is a perfect word, I assume :)15:26
danilosanyway, enough of that for now, those smarter about code-hosting in LP and bzr can comment on the bug and close it as invalid if needed; it'd still be nice to improve the experience for potential Go Launchpad users15:28
dimiternniemeyer: reviewed15:29
niemeyerdimitern: Cheers15:29
mgzsorry, http->https redirect first... then:15:29
mgz$ curl https://launchpad.net/goyaml/.bzr/branch/location15:29
niemeyerfwereade: Any idea of when we'll make log messages reasonable?15:38
danilosI am seeing a problem in environs/openstack/config_test.py configTest.check method: it doesn't seem to clean the state properly after a failing test: I expect first test to fail (I am just adding it before adding the code), but the second one shouldn't fail: if I move my new test to the end, second one doesn't fail anymore15:39
fwereadeniemeyer, ...damn, I have a draft email proposing a small doable card15:39
niemeyerfwereade: Just got a request by dimitern to uglify messages to conform to other ugly messages15:39
niemeyerfwereade: I'm doing it, but the output is comical15:39
niemeyer2013/04/09 12:39:56 INFO JUJU:juju:publish cmd/juju: charm published at 2013-04-01T15:53:05Z as cs:~niemeyer/precise/ubuntu-615:40
fwereadedanilos, that sounds like a problem to me15:40
dimiternniemeyer: i think a consistent source is better than half-baked better solution only at places15:40
niemeyerJUJU! JUJU! JUJU! I mean it, JUJU!15:40
niemeyerdimitern: I'm doing it..15:40
dimiternniemeyer: and i agree the format needs to change to cut the bullshit of the logs :)15:40
niemeyerdimitern: But I don't have to agree.. :)15:40
fwereadeniemeyer, the proposal will eliminate that at least -- thank you for bearing with us re consistency in the meantime, though15:40
danilosfwereade, http://pastebin.ubuntu.com/5692650/ and my test is in lp:~danilo/juju-core/bug-113533515:41
niemeyerfwereade: np.. I'm more bothered by presenting people with such obviously bad UX than the fact I have to change it15:42
dimiternfwereade: i though the JUJU: badge was supposed to somehow help the logs integrate better with syslog, but it's kinda crap if it's a requirement - a smart syslog will badge these according to sources15:43
fwereadedanilos, omg, that's evil15:43
fwereadedanilos, var credentialsTestConfig = configTests[12]15:43
danilosfwereade, aah15:44
fwereadedanilos, I haven't checked your branch, but I bet you inserted a test before that one, thus throwing off the hardcoded magiv number15:44
mgzyeah, that is good.15:44
fwereadedanilos, please kill that technique with fire if you would be so good :)15:44
danilosfwereade, I'd be delighted to :) any suggestions as to how since I am just getting to grips with go syntax? :)15:45
danilosfwereade, should I simply split it out into a separate var, and then reference it in the big array after that?15:45
mgzmove it above the block of test definitions as its own thing, then add it in the list15:45
fwereadedanilos, easiest way: just stick that test in a var, then stick the var into the []tests15:45
fwereadedanilos, but cast an eye over the actual uses of it15:45
danilosmgz, fwereade: ok, I guess we are all on the same page, thanks :)15:45
danilosfwereade, of course15:45
fwereadedanilos, it may be that it's not worth it in the first place ;)15:46
danilosfwereade, right, it's used in a single place with a different test.err and a couple of different env vars (just below the declarations): I'll see if I can move that as a separate entry in the configTests array15:48
dimiterntoday i tried the classic bootstrap+deploy mysql/wordpres+add-unit for both+expose; in ec2 - everything worked with the new provisioning!15:48
fwereadedanilos, great, thanks15:48
* fwereade cheers at dimitern15:49
* danilos tries to count from 0 to 12 in an array :)15:49
fwereadedimitern, https://codereview.appspot.com/8561044/ LGTM15:49
fwereadedimitern, btw, what's the value of BootstrapNonce?15:50
fwereadedimitern, didn't spot that15:50
dimiternfwereade: "user-admin:bootstrap", as you suggested - it was part of the previous CL15:50
fwereadedimitern, ah, great, thanks15:50
dimiternfwereade: thanks for the review; i really want to finish and land both of these today15:51
fwereadedimitern, what was the thinking behind UUID for nonce?15:52
fwereadedimitern, seems a shame not to have it badged with the provisioning machine15:53
fwereadedimitern, nothing wrong with a UUID in itself, but I'd prefer the nonce to look like "machine-0:<UUID>" if we're going with that rather than a sequence15:54
niemeyerpublish is going in!15:54
* fwereade cheers at niemeyer15:55
niemeyerand the server has been updated last night, which means it should work for everybody using tip already15:55
dimiternfwereade: fair enough, machine-0:<uuid> it is16:06
* rogpeppe eats the last piece of Easter chocolate16:06
TheMuerogpeppe: enjoy, i'll have dinner in a few moments. today no lunch. :/16:07
dimiterni need two LGTMs still on the following CLs: https://codereview.appspot.com/8561044/ and https://codereview.appspot.com/8561045/16:36
dimiternfwereade: ping for the second one16:38
rogpeppedimitern: looking16:39
rogpeppedimitern: i know that we have to set the nonce at the same time as the instance id, so SetProvisioned is reasonable there (i'm not keen on the name, but can't think of a better one currently), but i think InstanceId should just return the instance id, and a new method InstanceNonce (?) can return the nonce. no need to make calling instanceid harder, i think.16:41
rogpeppedimitern: it's a pity that SetInstanceId is used as a test example everywhere!16:42
dimiternrogpeppe: not keen on having a way to get the nonce directly, it's supposed to be internal16:43
rogpeppedimitern: huh? it's blatantly not internal - the function returns it!16:43
dimiternrogpeppe: I mean it's intentional not having a way to get it, you're supposed to check if it's sane with CheckProvisioned only16:44
rogpeppedimitern: another possibility would be to define InstanceId as struct {Id string; Nonce string}, i suppose16:44
rogpeppedimitern: in which case, don't return it from InstanceId16:44
dimiternrogpeppe: InstanceId returning a bool is kinda half-useful now, because we have CheckProvisioned, i agree16:45
dimiternrogpeppe: it's not returned from InstanceId16:45
rogpeppedimitern: ah, sorry, i misread16:46
dimiternrogpeppe: so InstanceId() -> InstanceId and that's it?16:47
rogpeppedimitern: i'm not sure i understand you there16:48
dimiternrogpeppe: if i understood you correctly, you're suggesting to change machine.InstanceId() to return InstanceId only, rather than InstanceId and bool (instance != "")?16:49
rogpeppedimitern: no, not really. i lost that argument aeons ago.16:49
rogpeppedimitern: i'd misinterpreted a change in your CL16:49
dimiternrogpeppe: we're living in a brave new world now, times for such changes, if they make sense16:50
rogpeppedimitern: i'll leave it for now16:50
dimiternrogpeppe: works for me16:50
rogpeppedimitern: hmm, i'm surprised tests pass. i thought there were probably various tests that assumed you could call SetInstanceId more than once16:56
dimiternrogpeppe: there was one in api_test, which I changed (TestMachineRefresh), to work like the similar test in state16:57
rogpeppedimitern: ah, i wondered why TestMachineRefresh had changed so much16:58
dimiternrogpeppe: and there was another one calling SetInstanceId(x, y) and then SetInstanceId("", ""), where I removed the second case, because the next case is SetAgentTools(), which should also trigger a change16:58
=== deryck_ is now known as deryck[lunch]
danilosTheMue: another one from me if you've got the time: https://codereview.appspot.com/8584043/17:23
danilosfwereade, I suppose you might want to comment on the general approach at least: https://codereview.appspot.com/8584043/17:24
dimiterndanilos: I think he signed off already, I can take a look17:24
danilosdimitern, sure, I'd appreciate it (this is just a refactor to fix problems I hit in the test when adding my own code)17:25
niemeyerTrivial one: https://codereview.appspot.com/858504317:27
dimiterndanilos: good change, but while you're at it, why not fix the source of the problem (ec2 provider, where this code was copied from)?17:30
mgzat least let him do that in a seperate mp :)17:30
danilosdimitern, heh, I did not know it was copied from there :)17:30
danilosdimitern, but yeah, let's do that in a separate MP, I want to get some traction on the actual bug fix as well17:31
dimiternmgz: well, it's all the same - the changes should be identical, and a bit less shit overall :) so win-win17:31
dimiterndanilos: at least put a TODO(danilos) Fix this test to save/restore env vars, like openstack config_test ?17:32
danilosdimitern, sure17:32
danilosdimitern, I can try seeing if it's simple enough (depending on how similar they are: there might be some intricate test details that I don't want to just do over in haste)17:33
dimiterndanilos: they should be almost identical, the check() func for sure17:34
dimiterndanilos: but a TODO is fine, lest we forget about it17:34
danilosdimitern, a bunch of variables are renamed (looking at the diff in trunk), even in the configTest struct17:36
dimiterndanilos: ok then, then it's better to leave it off for now17:36
dimiternniemeyer: reviewed17:43
dimiterndanilos: you too17:43
danilosdimitern, thanks17:43
niemeyerdimitern: Danke!17:43
danilosdimitern, for the "invalid-mode", do you want me to define a const in the test (since this is a non-existant auth mode, the value is typed in as a throw-away string)?17:46
dimiterndanilos: you can use AuthMode("invalid-mode") w/o problems, it'll be obvious even more I think17:47
danilosdimitern, ah, right, thanks17:48
dimiterndanilos: or maybe even without the cast - since literal string consts in go are casted automatically17:48
danilosdimitern, it is being used without the cast right now, so now I am not sure what do you mean?17:48
dimiterndanilos: when in doubt if anything will work, try it on https://play.golang.org/ - you can even share it :)17:49
danilosdimitern, you mean define it as a const in the test file and then use it there?17:49
dimiternoops - it's http: only, no s17:49
danilosdimitern, cool, nice tip, I'll see if it can replace  what python shell does for me in python17:49
dimiterndanilos: I mean try authMode: "invalid-mode", when authMode is typed, rather than just string - if it compiles, you're good, if not, cast it17:50
danilosdimitern, ah, ok17:50
dimiterndanilos: the best thing about go is most things are compile-time checked17:51
niemeyerdimitern: If I can explore you one last time today: https://codereview.appspot.com/858904317:52
niemeyerdimitern: Also tiny17:53
rogpeppeeod for me. g'night all.17:53
niemeyerrogpeppe: Cheers!17:53
dimiternniemeyer: sure, np17:53
rogpeppeniemeyer: nnight17:53
dimiternrogpeppe: night!17:53
niemeyerI believe this is the last piece I'll be doing on publish right now17:54
dimiternniemeyer: LGTM17:55
niemeyerdimitern: Super, thanks!17:55
niemeyerfwereade: Are you around by any chance?17:56
danilosdimitern, it compiles, but it ends up being AuthMode for the expected value, and string for the received value: I suppose config.go makes it as string17:58
dimiterndanilos: ah, bugger.. ok so reluctantly I'd go with strings then (why we have consts we're not using them also in tests? because the config stuff is crap basically..)18:00
danilosdimitern, right, but let's not fix all the problems in there today :)18:01
dimiterndanilos: indeed :)18:01
dimitern'night guys, i'm off18:02
danilosdimitern, night18:03
niemeyerdanilos: Wanna do a second review on those two CLs?  They're both tiny and easy going: https://codereview.appspot.com/8585043/, https://codereview.appspot.com/858904318:04
niemeyerWow.. look at those numbers, btw18:04
danilosniemeyer, yeah, very confusing18:05
danilosniemeyer, I am not sure I know how to use rietveld to review stuff already, but sure (if you'd trust a 2nd day Go expert like myself :)18:06
niemeyerdanilos: It's already the second review.. (or 3rd, in the case of the first change, as I talked to William about it ahead of time)18:06
niemeyerdanilos: So, sure, I'm happy to take your review on it18:06
niemeyerdanilos: Look at the CLs, if you have any comments/questions, double-click on the respective line and comment18:07
niemeyerdanilos: When you're done with the whole review, click on "Publish+Mail comments" at the top18:07
niemeyerdanilos: Include "LGTM" if you're happy with the changes to go in, or any other commnets18:07
danilosniemeyer, cool, I'll do them and you can be my second reviewer on https://codereview.appspot.com/8584043/ :)18:07
=== deryck[lunch] is now known as deryck
danilosniemeyer, LGTM on both, one minor comment on the first one18:16
niemeyerdanilos: LGTM as well, with a comment on the restoring18:18
niemeyerdanilos: Sensible comments, thakns.18:19
danilosniemeyer, nice, thanks for reminding me of the defer18:20
niemeyerdanilos: Yeah, it's a nice superpower we have in Go :)18:21
niemeyerdanilos: Note that it's not just convenience.. it solves an actual issue because there are possible paths before the current restoring that could exit the function without restoring18:21
danilosniemeyer, right, I can imagine other errors being caught and handled in the code in between18:22
* danilos -> off18:39
fwereadeniemeyer, hey, I'm back unless laura wakes back up19:50
niemeyerfwereade: Heya19:53
niemeyerfwereade: All good19:53
niemeyerfwereade: Already got reviews, and changes are in19:53
niemeyerfwereade: Including the validator changes we talked about19:53
fwereadeniemeyer, you rock19:54
fwereadeniemeyer, thank you very much :)19:54
=== gary_poster is now known as gary_poster|away
=== gary_poster|away is now known as gary_poster
niemeyerfwereade: It's been my pleasure20:46
niemeyerfwereade: Thanks for bearing with me as well :)20:46
=== markramm is now known as mramm_
* thumper frowns22:14
thumperfwereade: so... is there a go debugger that would allow me to step through a test?22:19
thumperto figure out what isn't working?22:19
thumpertest fails when I'd expect it to pass22:19
fwereadethumper, hmm, I am not a debugger person, so all I can do is try to hunt down the blog post I've seen people mentioning22:20
fwereadethumper, but if the symptoms are amenable to concise description they might ring a bell for me22:20
fwereadethumper, http://golang.org/doc/gdb22:21
thumperfwereade: well, we could chat about it, hangout would be faster22:21
fwereadethumper, sure22:22
thumperfwereade: I'll start it22:23
fwereadethumper, yes please, sorry22:23

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