[00:18] * thumper proposes IsTrue and IsFalse in a separate branch for wallyworld_ [00:18] \o/ [00:19] wallyworld_: I'll collect that beer sometime [00:19] ok [00:20] wallyworld_: I'm trying to think how to test the error conditions better [00:20] which branch? [00:23] thumper: afk for a few minutes, bathroom guy arrived [00:23] ack [00:23] wallyworld_: Rietveld: https://codereview.appspot.com/10395044 this one [00:24] davecheney: how do I create an inline []interface{} containing strings [00:24] ? [00:25] inline interface{} ? [00:25] nm [00:25] []interface{}{"foo", "bar"} [00:25] I was missing the first {} [00:51] 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 work [00:55] wallyworld_: I'm just making some lunch [00:55] but can talk soon [00:55] no hurry, whenever [00:55] now you've made me hungry [01:02] * thumper is making pizza [01:13] wallyworld_: chat now while the pizza cools? [01:19] thumper: i'm free now but feel free to eat first [01:21] * thumper is munching now [01:21] wallyworld_: I'll ping you when not chewing [01:21] sure [01:29] * thumper goes to make a coffee, then let the dog in [01:40] wallyworld_: now? [01:40] ok [01:40] i'll make a url [01:40] thumper: https://plus.google.com/hangouts/_/d3f48db1cccf0d24b0573a02f3a46f709af109a6 [03:55] * thumper needs another +1 [03:55] davecheney: you around? [03:56] bigjools: I wonder if your +1 is good enough :) [03:57] I think if I tried to propose this branch now, it'd shit itself [03:57] because I added a dependency [03:59] * thumper does it anyway to see what it looks like [04:21] thumper: +1 on what? [04:21] hi jam [04:21] jam: https://codereview.appspot.com/10395044/ [04:22] thumper: 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] *I* like modules, but I recognize it doesn't always need to be separated. [04:22] jam: yes... [04:22] because [04:22] import . "launchpad.net/juju-core/testing/checkers" [04:22] as it brings them into the current namespace [04:22] normally with the testing functions, we don't want that [04:23] only checkers are in the testing/checkers [04:23] so you shouldn't be bringing in other functions [04:23] means our checkers can be used just like the normal gocheck ones [04:23] (I don't think it would be terrible to have c.Assert(foo, testing.IsTrue) but I'll live with that) [04:24] jam: that makes it longer than c.Assert(foo, Equals, true) [04:24] :) [04:24] trying to make the tests more readable [04:24] thumper: I type at about 60wpm :). I would hope the point of IsTrue is to give better messages. [04:24] by having meaningful checkers wthout package prefixes [04:25] me too [04:25] I'm more about the readability of the code [04:25] and I think without the package prefix reads more nicely [04:25] jam: however there are also some we work with that don't seemed to have learned to type :-) [04:26] thumper: ack [04:26] sorry, was at lunch [04:26] davecheney: that's ok [04:27] and when I say 'was at lunch' [04:27] i was in my kitchen eating a sausage [04:32] thumper: reviewed https://codereview.appspot.com/10395044/ [04:32] davecheney: sounds healthy [04:32] jam: ta [04:36] jam: 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:37] thumper: vila had a patch which moved ignored files into a lost-and-found instead of marking it conflicted. [04:37] jam: now, I tend to go "clean-tree", followed by "resolve --all" [04:39] thumper: 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:40] * thumper nods [04:48] thumper: bzr.transform.orphan_policy=move [04:48] will put them in `bzr-orphans' [04:49] thanks to spiv in #bzr [05:37] jam: changed the file test functions into checkers [05:37] jam: have many more tests now [05:40] ok, and now I'm off [05:40] see y'all tomorrow === tasdomas_afk is now known as tasdomas === _mup__ is now known as _mup_ [07:47] mornin' all [08:08] morning guys [08:09] phew, 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. === 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/ [09:42] rogpeppe1: I'm looking at https://code.launchpad.net/%7Erogpeppe/juju-core/312-alt-api-jobs/+merge/170135 [09:42] your cover letter sounds like you were moving code [09:42] but I only see addition of code. [09:42] Am I missing something? [09:43] jam: looking [09:43] jam: state/apiserver/machine/machiner.go has moved from state/apiserver/machiner [09:44] jam: the machine agent API is new [09:45] jam: (as per the CL description) [09:45] jam: (well, CL title anyway) [09:48] rogpeppe1: https://codereview.appspot.com/10398043/ I guess codereview doesn't show renames very well. The only '-' lines I see are really machine_test.go [09:49] it would appear Launchpad was unable to produce a diff [09:49] jam: no, it doesn't know about renames at all [09:50] jam: 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 directoryu [09:50] jam: (note the package name change at the top) [09:51] rogpeppe1: well "machiner" vs "machine" is a bit hard to spot. You can actually see the rename in the unified diff mode. [09:52] rogpeppe1: is there more than just mechanical changes here? [09:52] jam: yes [09:52] rogpeppe1: any chance you can point them out, because most of the diff is just renaming stuff [09:53] new code: [09:53] https://codereview.appspot.com/10398043/diff/2001/state/api/params/params.go [09:53] https://codereview.appspot.com/10398043/diff/2001/state/apiserver/machine/agent.go [09:54] https://codereview.appspot.com/10398043/diff/2001/state/apiserver/machine/agent_test.go [09:54] rogpeppe1: 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] Is that %v vs %#v ? [09:54] jam: yes [09:54] jam: see the fmt docs for details [09:55] jam: that's all the significant new code. there are a few other tweaks here and there, which should be evident [09:56] jam: sorry for the confusion. i should probably have split it up into about 4 small CLs [09:56] jam: but i thought it was probably ok as is [09:56] rogpeppe1: it probably would have been a bit easirer to review. Mixing mechanical renames with functional changes takes different mindsets to review. [09:56] jam: agreed [09:57] The 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] do [09:57] jam: i was trying to make a CL that would be a replacement for an earlier CL with similar intent in the another pipeline [09:58] jam: but i totally failed to merge it [10:00] mgz, danilos: I'll definitely miss the standup today, have a kid's-friend's-birthday party to attend. [10:00] I've been mostly doing reviews, my code to mgo landed, though. [10:00] jam: okay [10:04] rogpeppe1: reviewed [10:04] LGTM [10:04] jam: thanks [10:11] jam, 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 overnight [10:11] jam, what did tim do, and what should I do to induce the same thing to happen? [10:15] jam: ok [10:16] jam: don't drink too much with those kids, though :) === tasdomas is now known as tasdomas_afk [10:25] fwereade: hi, you up for a chat sometime? [10:25] wallyworld, sure, would you start one? [10:25] ok [10:25] fwereade: https://plus.google.com/hangouts/_/d3f48db1cccf0d24b0573a02f3a46f709af109a6 [10:27] danilos: any chance of a review of https://codereview.appspot.com/10398043/ please? [10:27] rogpeppe3, sure, taking a look [10:28] danilos: thanks === tasdomas_afk is now known as tasdomas === TheRealMue is now known as TheMue [10:50] rogpeppe3, LGTM, minor comments on your comments only :) [10:51] danilos: tyvm [10:51] rogpeppe3, I like it how your code looks nice and clean, good job :) [10:51] danilos: thanks [10:53] danilos: 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] danilos: so there's no possible latency to be saved AFAICS [10:54] danilos: we can only ever usefully pass a single id to GetMachines [11:00] rogpeppe3, right, I'd say the API call names are a bit misleading then [11:00] rogpeppe3, I was thinking of something like juju-gui using a call like this to get information on all the machines [11:01] danilos: we are directed to make bulk API entry points for every single API call, regardless of whether it's actually appropriate to have one [11:01] danilos: i must obey :-) [11:01] rogpeppe3, ah, I see your point: you are complaining about the policy in that comment, understood :) === rogpeppe3 is now known as rogpeppe [11:01] rogpeppe, ok then, keep the 'utter maddness' in :P [11:03] danilos: the madness remains :-) === tasdomas is now known as tasdomas_afk === tasdomas_afk is now known as tasdomas [11:33] mgz, hey-hey, joining us? [11:33] danilos: want to talk? I'd need to reboot to do g+ [11:33] had, same time [11:34] can you guys do mumble for today? [11:34] ok [11:34] mgz, coming [11:36] TheMue, ping [11:36] wallyworld__, can you hear me? [11:45] doh! [11:45] I'd left it on push to tal [11:47] danilos, 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:53] fwereade: pong [11:53] TheMue, tyvm for sync-tools work; can I ask for a little followup though? [11:53] fwereade: sure, just proposed the resumer with better testing ;) [11:54] TheMue, 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:55] TheMue, (moving the actual syncing off somewhere different should also be done soon, but that's not what I'm talking about today) [11:55] fwereade: ok, but in ec2 it will be public [11:56] fwereade: thought i had the source stuff removed everywhere *sigh* will cleanup a bit :D [11:56] TheMue, I want it to be public, it's a perfectly good StorageReader ;p [11:56] TheMue, I think it's just one var :) [11:56] fwereade: fine [11:59] mgz, heh, that only means you could have replied to all of our trash talk, but you missed it :P [12:00] TheMue, btw, I redid https://codereview.appspot.com/10302043 to be simpler, would you take a quick look please? [12:00] fwereade: *click* [12:04] danilos: 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:05] fwereade: nice, looks better that way [12:08] rvba, sure [12:08] ta [12:43] right, away with dodgy phone connections; i now have wired ether to the garden! === wedgwood_away is now known as wedgwood === BradCrittenden is now known as bac [13:08] * rogpeppe goes for some lunch [13:18] hey guys, got this error updating trunk just now: [13:18] imports launchpad.net/juju-core/state/apiserver/machiner: import "launchpad.net/juju-core/state/apiserver/machiner": cannot find package [13:22] now it's working again, nm then === BradCrittenden is now known as bac [13:44] gary_poster: I'll see if I can help him. [13:45] benji, oh ye of the jumping channels, thank you very much! [13:45] gary_poster: it's confusing being omnipresent [13:45] lol [14:06] fwereade: ping, hangout [14:11] jam: ping [14:13] niemeyer: jam probably won't be around this afternoon now [14:16] mgz: Cool, thanks [14:16] rogpeppe: ping [14:16] niemeyer: in a call right now [14:16] rogpeppe: Cool, cheers [14:16] niemeyer: i'll be with you in a bit [14:17] rogpeppe: Thanks, not in a hurry [14:35] niemeyer: hiya [14:35] niemeyer: call done [14:35] rogpeppe: Heya [14:36] rogpeppe: Would you mind to have a quick look at https://codereview.appspot.com/10366044/ [14:36] niemeyer: looking [14:36] rogpeppe: It's a simple change to implement ,inline in goyaml [14:36] rogpeppe: It's pretty much copied from mgo/bson [14:36] rogpeppe: There's a second part that I didn't do yet, to support inlining of maps [14:36] rogpeppe: I'll do that as well, but that will require some additional time [14:38] rogpeppe: I've just submitted https://codereview.appspot.com/10417043 as well, which is an obvious fix [14:43] niemeyer: what happens if there are field conflicts? [14:43] rogpeppe: The same thing that happens currently: an error [14:43] rogpeppe: I know that's unlike the json API.. but it's okay, we can improve it later in a compatible way [14:43] niemeyer: sounds like a good way forward [14:45] Erm [14:45] Weird those harsh disconnections [14:46] niemeyer: sounds like a good way forward [14:46] rogpeppe: That was the last thing I saw [14:46] niemeyer_: that's the last thing i said :-) [14:46] Cool [14:49] niemeyer_: do anonymous struct members work essentially the same way as "inline" struct members? [14:49] WTF? [14:51] [15:49:08] niemeyer_: do anonymous struct members work essentially the same way as "inline" struct members? [14:52] niemeyer__: i don't seem to see any code that understands about anonymous struct members, so perhaps not? [14:52] rogpeppe: No, the only way to inline is with ,inline [14:52] niemeyer__: ah [14:52] rogpeppe: They are orthogonal concerns [14:52] niemeyer__: encoding/json doesn't seem to think so [14:53] rogpeppe: mgo/bson seems to think so, though :) [14:53] rogpeppe: It's a departure from the standard behavior for sure, and a good one IMO [14:54] niemeyer__: i'm wondering when you wouldn't want an embedded struct member to be treated as inline [14:54] rogpeppe: They are orthogonal concerns [14:54] niemeyer__: they *can* be orthogonal, sure. i'm just wondering if there's a particular benefit in having things that way [14:54] 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 file [14:55] rogpeppe: This *is* the benefit [14:56] niemeyer__: perhaps so. i'm happy to see how it pans out. [14:57] rogpeppe: Either way, we don't really have to debate this [14:57] rogpeppe: We can't do anything else wihtout breaking compatibility [14:57] niemeyer__: good point [14:58] niemeyer__: i think you should be able to inline a *struct too [14:59] niemeyer__: ha [14:59] [15:57:49] niemeyer__: good point [14:59] [15:58:31] niemeyer__: i think you should be able to inline a *struct too [15:30] niemeyer__: reviewed, BTW === tasdomas is now known as tasdomas_afk [15:57] fwereade_: ping === mgz_ is now known as mgz [16:05] rogpeppe, pong [16:05] fwereade_: i was just looking at this line in state/api/state.go: [16:06] func (st *State) Machiner() (*machiner.Machiner, error) [16:06] fwereade_: and thinking about future developments. [16:06] rogpeppe, oh yes [16:06] fwereade_: i'm wondering about something like this instead: func (st *State) Machiner() *machiner.State [16:07] fwereade_: reasoning that making a dummy request to check if we're allowed access isn't really that important [16:07] fwereade_: and that from a client's point of view, it really looks like a kind of state object [16:07] rogpeppe, assuming that's all handled by the root swapping, yeah [16:07] fwereade_: then we can have machineagent.State, upgrader.State etc etc [16:08] rogpeppe, it kinda does, yeah, I'm still thinking though [16:08] fwereade_: well, you'll still be able to call State.Machiner even if you're not a machine agewnt [16:08] fwereade_: but i don't think it matters much if we don't get an error until we make the first request [16:09] fwereade_: because it's something that should never happen in practice anyway [16:09] rogpeppe, sure; and that leaves us open to construct agent-specific facades if we decide we have leisure to be clever in the future [16:09] fwereade_: yeah [16:10] fwereade_: i might be heading in that direction anyway [16:10] rogpeppe, I'm kinda +1 on having a common name, so the shapes of the packages are nice and clear and consistent [16:11] rogpeppe, but I'm not quite sure it should be State [16:11] rogpeppe, machiner.API, for example, feels closer than machiner.State [16:11] fwereade_: using the name "State" means that we won't have to rename all the existing variables in the agent code [16:11] rogpeppe, ha, good point :) [16:12] 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 ;p [16:13] fwereade_: i think that from a client point of view, it's actually a perfectly sensible name - that's how you talk to the state [16:13] fwereade_: the API is just a window onto the state [16:13] rogpeppe, you may very well be right [16:13] rogpeppe, cheers [16:13] * fwereade_ is off for a bit now, probably back later at some point [16:14] fwereade_: ok, thanks [17:32] time to stop and enjoy the sunshine with a cold drink [17:33] g'night all === niemeyer__ is now known as niemeyer [20:34] I'm going to be starting a little late today as I have to take kids to the dentist [22:16] Hi folks: found one that might be low-hanging-fruit: https://bugs.launchpad.net/juju-core/+bug/1192728 [22:16] <_mup_> Bug #1192728: differing behavior with pyjuju: config-get all json === wedgwood is now known as wedgwood_away [23:44] jam: since you commented on my branch, would you be willing to vote for landing? It's this one: https://codereview.appspot.com/10367045/ [23:45] If so, I can apply your generic version of the locking-test pattern in a follow-up branch. [23:45] (For now, we're rather blocked on this branch)