/srv/irclogs.ubuntu.com/2013/06/19/#juju-dev.txt

* thumper proposes IsTrue and IsFalse in a separate branch for wallyworld_00:18
wallyworld_\o/00:18
thumperwallyworld_: I'll collect that beer sometime00:19
wallyworld_ok00:19
thumperwallyworld_: I'm trying to think how to test the error conditions better00:20
wallyworld_which branch?00:20
wallyworld_thumper: afk for a few minutes, bathroom guy arrived00:23
thumperack00:23
thumperwallyworld_: Rietveld: https://codereview.appspot.com/10395044 this one00:23
thumperdavecheney: how do I create an inline []interface{} containing strings00:24
thumper?00:24
davecheneyinline interface{} ?00:25
thumpernm00:25
thumper[]interface{}{"foo", "bar"}00:25
thumperI was missing the first {}00:25
wallyworld_thumper: when you are free, i'd like to hear about the outcome of your deliberations regarding the lxc branch. but don't interrupt your current work00:51
thumperwallyworld_: I'm just making some lunch00:55
thumperbut can talk soon00:55
wallyworld_no hurry, whenever00:55
wallyworld_now you've made me hungry00:55
* thumper is making pizza01:02
thumperwallyworld_: chat now while the pizza cools?01:13
wallyworld_thumper: i'm free now but feel free to eat first01:19
* thumper is munching now01:21
thumperwallyworld_: I'll ping you when not chewing01:21
wallyworld_sure01:21
* thumper goes to make a coffee, then let the dog in01:29
thumperwallyworld_: now?01:40
wallyworld_ok01:40
wallyworld_i'll make a url01:40
wallyworld_thumper: https://plus.google.com/hangouts/_/d3f48db1cccf0d24b0573a02f3a46f709af109a601:40
* thumper needs another +103:55
thumperdavecheney: you around?03:55
thumperbigjools: I wonder if your +1 is good enough :)03:56
thumperI think if I tried to propose this branch now, it'd shit itself03:57
thumperbecause I added a dependency03:57
* thumper does it anyway to see what it looks like03:59
jamthumper: +1 on what?04:21
thumperhi jam04:21
thumperjam: https://codereview.appspot.com/10395044/04:21
jamthumper: my immediate thought is, do we need 'testing/checkers' vs just having the checkers in the 'testing' package? (given the earlier conversations about this).04:22
jam*I* like modules, but I recognize it doesn't always need to be separated.04:22
thumperjam: yes...04:22
thumperbecause04:22
thumperimport . "launchpad.net/juju-core/testing/checkers"04:22
thumperas it brings them into the current namespace04:22
thumpernormally with the testing functions, we don't want that04:22
thumperonly checkers are in the testing/checkers04:23
thumperso you shouldn't be bringing in other functions04:23
thumpermeans our checkers can be used just like the normal gocheck ones04:23
jam(I don't think it would be terrible to have c.Assert(foo, testing.IsTrue) but I'll live with that)04:23
thumperjam: that makes it longer than c.Assert(foo, Equals, true)04:24
thumper:)04:24
thumpertrying to make the tests more readable04:24
jamthumper: I type at about 60wpm :). I would hope the point of IsTrue is to give better messages.04:24
thumperby having meaningful checkers wthout package prefixes04:24
thumperme too04:25
thumperI'm more about the readability of the code04:25
thumperand I think without the package prefix reads more nicely04:25
thumperjam: however there are also some we work with that don't seemed to have learned to type :-)04:25
davecheneythumper: ack04:26
davecheneysorry, was at lunch04:26
thumperdavecheney: that's ok04:26
davecheneyand when I say 'was at lunch'04:27
davecheneyi was in my kitchen eating a sausage04:27
jamthumper: reviewed https://codereview.appspot.com/10395044/04:32
thumperdavecheney: sounds healthy04:32
thumperjam: ta04:32
thumperjam: did switch ever get to the state where it could remove directories that were added  in the other branch, and only contain ignored files?04:36
jamthumper: vila had a patch which moved ignored files into a lost-and-found instead of marking it conflicted.04:37
thumperjam: now, I tend to go "clean-tree", followed by "resolve --all"04:37
jamthumper: I think you have to turn it on with a config setting (tell it to treat ignored as lost and put them there, but I don't remember the exact syntax)04:39
* thumper nods04:40
jamthumper: bzr.transform.orphan_policy=move04:48
jamwill put them in `bzr-orphans'04:48
jamthanks to spiv in #bzr04:49
thumperjam: changed the file test functions into checkers05:37
thumperjam: have many more tests now05:37
thumperok, and now I'm off05:40
thumpersee y'all tomorrow05:40
=== tasdomas_afk is now known as tasdomas
=== _mup__ is now known as _mup_
rogpeppemornin' all07:47
TheMuemorning guys08:08
TheMuephew, what a start. had to fetch our daughter from work due to circulation problems. weather is extreme close and will go up to 34° today.08:09
=== danilos__ is now known as danilos
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: danilos | Bugs: 10 Critical, 79 High - https://bugs.launchpad.net/juju-core/
jamrogpeppe1: I'm looking at https://code.launchpad.net/%7Erogpeppe/juju-core/312-alt-api-jobs/+merge/17013509:42
jamyour cover letter sounds like you were moving code09:42
jambut I only see addition of code.09:42
jamAm I missing something?09:42
rogpeppe1jam: looking09:43
rogpeppe1jam: state/apiserver/machine/machiner.go has moved from state/apiserver/machiner09:43
rogpeppe1jam: the machine agent API is new09:44
rogpeppe1jam: (as per the CL description)09:45
rogpeppe1jam: (well, CL title anyway)09:45
jamrogpeppe1: https://codereview.appspot.com/10398043/ I guess codereview doesn't show renames very well. The only '-' lines I see are really machine_test.go09:48
jamit would appear Launchpad was unable to produce a diff09:49
rogpeppe1jam: no, it doesn't know about renames at all09:49
rogpeppe1jam: if you look at https://codereview.appspot.com/10398043/diff/2001/state/apiserver/machine/machiner.go - all that "unchanged" code is moved from a different directoryu09:50
rogpeppe1jam: (note the package name change at the top)09:50
jamrogpeppe1: well "machiner" vs "machine" is a bit hard to spot. You can actually see the rename in the unified diff mode.09:51
jamrogpeppe1: is there more than just mechanical changes here?09:52
rogpeppe1jam: yes09:52
jamrogpeppe1: any chance you can point them out, because most of the diff is just renaming stuff09:52
rogpeppe1new code:09:53
rogpeppe1https://codereview.appspot.com/10398043/diff/2001/state/api/params/params.go09:53
rogpeppe1https://codereview.appspot.com/10398043/diff/2001/state/apiserver/machine/agent.go09:53
rogpeppe1https://codereview.appspot.com/10398043/diff/2001/state/apiserver/machine/agent_test.go09:54
jamrogpeppe1: what is the difference between GoStringer and Stringer? (I realize what the actual attribute difference means, but what is the functional difference between having a String method and a GoString method) ?09:54
jamIs that %v vs %#v ?09:54
rogpeppe1jam: yes09:54
rogpeppe1jam: see the fmt docs for details09:54
rogpeppe1jam: that's all the significant new code. there are a few other tweaks here and there, which should be evident09:55
rogpeppe1jam: sorry for the confusion. i should probably have split it up into about 4 small CLs09:56
rogpeppe1jam: but i thought it was probably ok as is09:56
jamrogpeppe1: it probably would have been a bit easirer to review. Mixing mechanical renames with functional changes takes different mindsets to review.09:56
rogpeppe1jam: agreed09:56
jamThe former is "is this a reasonable rename to do, yes/no" and the latter takes thinking about what the code is trying to doe.09:57
jamdo09:57
rogpeppe1jam: i was trying to make a CL that would be a replacement for an earlier CL with similar intent in the another pipeline09:57
rogpeppe1jam: but i totally failed to merge it09:58
jammgz, danilos: I'll definitely miss the standup today, have a kid's-friend's-birthday party to attend.10:00
jamI've been mostly doing reviews, my code to mgo landed, though.10:00
mgzjam: okay10:00
jamrogpeppe1: reviewed10:04
jamLGTM10:04
rogpeppe1jam: thanks10:04
fwereadejam, last night go-bot complained that I'd approved a branch without getting it re-reviewed... and then it somehow got magically merged by tim overnight10:11
fwereadejam, what did tim do, and what should I do to induce the same thing to happen?10:11
danilosjam: ok10:15
danilosjam: don't drink too much with those kids, though :)10:16
=== tasdomas is now known as tasdomas_afk
wallyworldfwereade: hi, you up for a chat sometime?10:25
fwereadewallyworld, sure, would you start one?10:25
wallyworldok10:25
wallyworldfwereade: https://plus.google.com/hangouts/_/d3f48db1cccf0d24b0573a02f3a46f709af109a610:25
rogpeppe3danilos: any chance of a review of https://codereview.appspot.com/10398043/ please?10:27
danilosrogpeppe3, sure, taking a look10:27
rogpeppe3danilos: thanks10:28
=== tasdomas_afk is now known as tasdomas
=== TheRealMue is now known as TheMue
danilosrogpeppe3, LGTM, minor comments on your comments only :)10:50
rogpeppe3danilos: tyvm10:51
danilosrogpeppe3, I like it how your code looks nice and clean, good job :)10:51
rogpeppe3danilos: thanks10:51
rogpeppe3danilos: re the bulk API call - it purports to allow you to get information for many machines at once. but we only allow a single machine to be got (the machine agent's own machine instance)10:53
rogpeppe3danilos: so there's no possible latency to be saved AFAICS10:53
rogpeppe3danilos: we can only ever usefully pass a single id to GetMachines10:54
danilosrogpeppe3, right, I'd say the API call names are a bit misleading then11:00
danilosrogpeppe3, I was thinking of something like juju-gui using a call like this to get information on all the machines11:00
rogpeppe3danilos: we are directed to make bulk API entry points for every single API call, regardless of whether it's actually appropriate to have one11:01
rogpeppe3danilos: i must obey :-)11:01
danilosrogpeppe3, ah, I see your point: you are complaining about the policy in that comment, understood :)11:01
=== rogpeppe3 is now known as rogpeppe
danilosrogpeppe, ok then, keep the 'utter maddness' in :P11:01
rogpeppedanilos: the madness remains :-)11:03
=== tasdomas is now known as tasdomas_afk
=== tasdomas_afk is now known as tasdomas
danilosmgz, hey-hey, joining us?11:33
mgzdanilos: want to talk? I'd need to reboot to do g+11:33
mgzhad, same time11:33
mgzcan you guys do mumble for today?11:34
wallyworld__ok11:34
danilosmgz, coming11:34
fwereadeTheMue, ping11:36
daniloswallyworld__, can you hear me?11:36
mgzdoh!11:45
mgzI'd left it on push to tal11:45
mgzdanilos, wallyworld__: my audio was working fine the whole time, I just didn't look at the right setting till just after we'd finished :)11:47
TheMuefwereade: pong11:53
fwereadeTheMue, tyvm for sync-tools work; can I ask for a little followup though?11:53
TheMuefwereade: sure, just proposed the resumer with better testing ;)11:53
fwereadeTheMue, please delete the dead source environment config, and move the storage reader into environs/ec2 (actual location open to discussion, but it shouldn't be in juju/cmd)11:54
fwereadeTheMue, (moving the actual syncing off somewhere different should also be done soon, but that's not what I'm talking about today)11:55
TheMuefwereade: ok, but in ec2 it will be public11:55
TheMuefwereade: thought i had the source stuff removed everywhere *sigh* will cleanup a bit :D11:56
fwereadeTheMue, I want it to be public, it's a perfectly good StorageReader ;p11:56
fwereadeTheMue, I think it's just one var :)11:56
TheMuefwereade: fine11:56
danilosmgz, heh, that only means you could have replied to all of our trash talk, but you missed it :P11:59
fwereadeTheMue, btw, I redid https://codereview.appspot.com/10302043 to be simpler, would you take a quick look please?12:00
TheMuefwereade: *click*12:00
rvbadanilos: Hi, would you mind having a look at jtv's branch (https://codereview.appspot.com/10367045/) when you get a chance?  Jeroen is off today but he will be back tomorrow and this is an important building block for our work on the Azure provider.12:04
TheMuefwereade: nice, looks better that way12:05
danilosrvba, sure12:08
rvbata12:08
rogpepperight, away with dodgy phone connections; i now have wired ether to the garden!12:43
=== wedgwood_away is now known as wedgwood
=== BradCrittenden is now known as bac
* rogpeppe goes for some lunch13:08
ahasenackhey guys, got this error updating trunk just now:13:18
ahasenackimports launchpad.net/juju-core/state/apiserver/machiner: import "launchpad.net/juju-core/state/apiserver/machiner": cannot find package13:18
ahasenacknow it's working again, nm then13:22
=== BradCrittenden is now known as bac
benjigary_poster: I'll see if I can help him.13:44
gary_posterbenji, oh ye of the jumping channels, thank you very much!13:45
benjigary_poster: it's confusing being omnipresent13:45
gary_posterlol13:45
TheMuefwereade: ping, hangout14:06
niemeyerjam: ping14:11
mgzniemeyer: jam probably won't be around this afternoon now14:13
niemeyermgz: Cool, thanks14:16
niemeyerrogpeppe: ping14:16
rogpeppeniemeyer: in a call right now14:16
niemeyerrogpeppe: Cool, cheers14:16
rogpeppeniemeyer: i'll be with you in a bit14:16
niemeyerrogpeppe: Thanks, not in a hurry14:17
rogpeppeniemeyer: hiya14:35
rogpeppeniemeyer: call done14:35
niemeyerrogpeppe: Heya14:35
niemeyerrogpeppe: Would you mind to have a quick look at https://codereview.appspot.com/10366044/14:36
rogpeppeniemeyer: looking14:36
niemeyerrogpeppe: It's a simple change to implement ,inline in goyaml14:36
niemeyerrogpeppe: It's pretty much copied from mgo/bson14:36
niemeyerrogpeppe: There's a second part that I didn't do yet, to support inlining of maps14:36
niemeyerrogpeppe: I'll do that as well, but that will require some additional time14:36
niemeyerrogpeppe: I've just submitted https://codereview.appspot.com/10417043 as well, which is an obvious fix14:38
rogpeppeniemeyer: what happens if there are field conflicts?14:43
niemeyerrogpeppe: The same thing that happens currently: an error14:43
niemeyerrogpeppe: I know that's unlike the json API.. but it's okay, we can improve it later in a compatible way14:43
rogpeppeniemeyer: sounds like a good way forward14:43
niemeyer_Erm14:45
niemeyer_Weird those harsh disconnections14:45
niemeyer_<rogpeppe> niemeyer: sounds like a good way forward14:46
niemeyer_rogpeppe: That was the last thing I saw14:46
rogpeppeniemeyer_: that's the last thing i said :-)14:46
niemeyer_Cool14:46
rogpeppeniemeyer_: do anonymous struct members work essentially the same way as "inline" struct members?14:49
niemeyer__WTF?14:49
rogpeppe[15:49:08] <rogpeppe> niemeyer_: do anonymous struct members work essentially the same way as "inline" struct members?14:51
rogpeppeniemeyer__: i don't seem to see any code that understands about anonymous struct members, so perhaps not?14:52
niemeyer__rogpeppe: No, the only way to inline is with ,inline14:52
rogpeppeniemeyer__: ah14:52
niemeyer__rogpeppe: They are orthogonal concerns14:52
rogpeppeniemeyer__: encoding/json doesn't seem to think so14:52
niemeyer__rogpeppe: mgo/bson seems to think so, though :)14:53
niemeyer__rogpeppe: It's a departure from the standard behavior for sure, and a good one IMO14:53
rogpeppeniemeyer__: i'm wondering when you wouldn't want an embedded struct member to be treated as inline14:54
niemeyer__rogpeppe: They are orthogonal concerns14:54
rogpeppeniemeyer__: they *can* be orthogonal, sure. i'm just wondering if there's a particular benefit in having things that way14:54
niemeyer__rogpeppe: Whether you want to handle something in your code as inlined or not is an independent concern of whether you want it to look like being in a single block in a file14:54
niemeyer__rogpeppe: This *is* the benefit14:55
rogpeppeniemeyer__: perhaps so. i'm happy to see how it pans out.14:56
niemeyer__rogpeppe: Either way, we don't really have to debate this14:57
niemeyer__rogpeppe: We can't do anything else wihtout breaking compatibility14:57
rogpeppeniemeyer__: good point14:57
rogpeppeniemeyer__: i think you should be able to inline a *struct too14:58
rogpeppeniemeyer__: ha14:59
rogpeppe[15:57:49] <rogpeppe> niemeyer__: good point14:59
rogpeppe[15:58:31] <rogpeppe> niemeyer__: i think you should be able to inline a *struct too14:59
rogpeppeniemeyer__: reviewed, BTW15:30
=== tasdomas is now known as tasdomas_afk
rogpeppefwereade_: ping15:57
=== mgz_ is now known as mgz
fwereade_rogpeppe, pong16:05
rogpeppefwereade_: i was just looking at this line in state/api/state.go:16:05
rogpeppefunc (st *State) Machiner() (*machiner.Machiner, error)16:06
rogpeppefwereade_: and thinking about future developments.16:06
fwereade_rogpeppe, oh yes16:06
rogpeppefwereade_: i'm wondering about something like this instead: func (st *State) Machiner() *machiner.State16:06
rogpeppefwereade_: reasoning that making a dummy request to check if we're allowed access isn't really that important16:07
rogpeppefwereade_: and that from a client's point of view, it really looks like a kind of state object16:07
fwereade_rogpeppe, assuming that's all handled by the root swapping, yeah16:07
rogpeppefwereade_: then we can have machineagent.State, upgrader.State etc etc16:07
fwereade_rogpeppe, it kinda does, yeah, I'm still thinking though16:08
rogpeppefwereade_: well, you'll still be able to call State.Machiner even if you're not a machine agewnt16:08
rogpeppefwereade_: but i don't think it matters much if we don't get an error until we make the first request16:08
rogpeppefwereade_: because it's something that should never happen in practice anyway16:09
fwereade_rogpeppe, sure; and that leaves us open to construct agent-specific facades if we decide we have leisure to be clever in the future16:09
rogpeppefwereade_: yeah16:09
rogpeppefwereade_: i might be heading in that direction anyway16:10
fwereade_rogpeppe, I'm kinda +1 on having a common name, so the shapes of the packages are nice and clear and consistent16:10
fwereade_rogpeppe, but I'm not quite sure it should be State16:11
fwereade_rogpeppe, machiner.API, for example, feels closer than machiner.State16:11
rogpeppefwereade_: using the name "State" means that we won't have to rename all the existing variables in the agent code16:11
fwereade_rogpeppe, ha, good point :)16:11
fwereade_rogpeppe, ok, sgtm, with an option on a future declaration (once I've forgotten this discussion) that it's a silly name and we should do a global replace ;p16:12
rogpeppefwereade_: i think that from a client point of view, it's actually a perfectly sensible name - that's how you talk to the state16:13
rogpeppefwereade_: the API is just a window onto the state16:13
fwereade_rogpeppe, you may very well be right16:13
fwereade_rogpeppe, cheers16:13
* fwereade_ is off for a bit now, probably back later at some point16:13
rogpeppefwereade_: ok, thanks16:14
rogpeppetime to stop and enjoy the sunshine with a cold drink17:32
rogpeppeg'night all17:33
=== niemeyer__ is now known as niemeyer
thumperI'm going to be starting a little late today as I have to take kids to the dentist20:34
dpb1Hi folks: found one that might be low-hanging-fruit: https://bugs.launchpad.net/juju-core/+bug/119272822:16
_mup_Bug #1192728: differing behavior with pyjuju: config-get all json <juju-core:New> <https://launchpad.net/bugs/1192728>22:16
=== wedgwood is now known as wedgwood_away
jtvjam: since you commented on my branch, would you be willing to vote for landing?  It's this one: https://codereview.appspot.com/10367045/23:44
jtvIf so, I can apply your generic version of the locking-test pattern in a follow-up branch.23:45
jtv(For now, we're rather blocked on this branch)23:45

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