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