=== slank is now known as slank_away === _mup__ is now known as _mup_ [07:40] davecheney, fwereade_, rogpeppe: Morning [07:40] TheMue, everybody, heyhey [08:14] morning all! [08:22] rogpeppe: hiya [08:42] * TheMue enjoys installing a fresh environment to run MAAS in KVM [08:57] moin. [09:10] morning! [09:30] aram, dimitern: hiya [09:30] TheMue: yo :) [10:25] fwereade_: you have reviews [10:25] fwereade_: i'd appreciate a look at https://codereview.appspot.com/7105054/ [10:27] rogpeppe, cool, thanks; I'll take a peek [10:45] rogpeppe, LGTM [10:45] fwereade_: thanks! [10:46] rogpeppe, I need to take a break for a bit... would you let http://paste.ubuntu.com/1533914/ roll around your mind for sanity for a bit? [10:46] fwereade_: looking [10:57] * jam goes to grab a coffee, will be back for standup in 2 min [11:01] mgz: poke [11:01] hey [11:01] for great mumbling! [11:05] I'm on. [11:14] niemeyer, heyhey [11:28] fwereade_: How's tricks? :) [11:28] niemeyer, good thanks; but a massive storm has just blown up here, somewhat to my surprise [11:29] fwereade_: Ugh [11:29] fwereade_: I guess you have good power rails there :) [11:30] niemeyer: hiya! [11:30] niemeyer, it usually messes with the wifi more than the power but power cuts are not unheard of here [11:30] holy crap, hail [11:38] lunchtime [11:40] fwereade_: Wow [11:40] niemeyer, and now it's just eerie silence [11:46] fwereade_: I'm always hoping for a good, massive hailstorm, just to see how bad it gets here, but alas - nothing worthy so far :D [11:48] dimitern, yeah, that wasn't all that much to write home about on reflection [11:48] dimitern, but still pretty unexpected, given the beautiful sunshine not half an hour before [11:48] dimitern, I think I just heard thunder though [11:50] fwereade_: yeah, this here is like mountain weather - 5m and it's completely different [11:50] rogpeppe, any thoughts re sanity of that paste? [11:51] fwereade_: what's the difference between Destroy and Remove? [11:51] rogpeppe, Destroy is the method everything has; it might set Dying or it might remove [11:52] rogpeppe, Remove only applies to units/machines because they're the only ones that follow the path we originally understood [11:52] rogpeppe, Dying services and relations are removed as side-effects of either Unit.Remove or RelationUnit.LeaveScope [11:53] rogpeppe, did you get a chance to take a look at death-and-destruction.txt? [11:54] fwereade_: yes, i did, although i wasn't sure how much is what we've got now, and how much is plans for the future. [11:54] rogpeppe, everything except how services are done, which was agreed before xmas and which I'm working on now [11:56] fwereade_: BTW why might Remove of a unit also remove its service? don't we allow services with no units? [11:56] rogpeppe, if the service is Dying, then the disappearing unit might be the last reference to the service [11:56] fwereade_: ah, of course [11:57] fwereade_: the paste looks reasonable to me [11:57] fwereade_: it'll be worth discussing this with niemeyer, since he's around [11:58] rogpeppe, quite so; niemeyer, I'd like to have an API discussion with you when you have some free time [11:59] fwereade_: Sure, any time [11:59] fwereade_: Next meeting is in an hour [11:59] niemeyer, I'd quite like a bite to eat before then -- could we do it right afterwards please? [12:00] niemeyer, (or, hmm, will that hit your lunchtime?) [12:00] fwereade_: Sounds good [12:00] niemeyer, ok, cool, thanks :) [12:00] fwereade_: It might, but it's okay either way.. worst case we do when I'm back [12:06] fairly simple CL if anyone cares to have a look: https://codereview.appspot.com/7085062 [12:17] rogpeppe: Looking [12:22] niemeyer: thanks [12:51] i keep getting error: environment "canonistack" has an unknown provider type "openstack" - what should I do to fix this? I tried go installing cmd/juju to make sure it was compiled with the latest source, still no luck [12:55] rogpeppe, fwereade_- any hints? ^^ [12:56] dimitern: juju needs to import the openstack provider [12:56] dimitern: import _ "launchpad.net/juju-core/environs/openstack" [12:56] rogpeppe: so we need that for all providers? even though they are registered? [12:57] dimitern: they can only register if they're imported [12:57] dimitern: (by at least one thing) [12:57] rogpeppe: I see, 10x [13:17] TheMue: https://wiki.canonical.com/Launchpad/MAASTesting I'm not sure if you knew of that one, but it should have some stuff about the Lenovo QA Lab for MaaS [13:17] thx [13:18] TheMue: I think rvba in #maas would probably be able to help out on questions/access information to the lab. [13:19] aram: ^^ probably you're interested in that one as well (if you haven't seen it already) === niemeyer_ is now known as niemeyer [13:43] fwereade_: Sorry, net hiccup [13:43] niemeyer, np, hangout is still open [13:43] fwereade_: Are you still in the same hangout? [13:43] fwereade_: Joining [13:50] wallyworld__: shall we try to sort out the public storage thing? [13:51] rogpeppe: would love to but it's almost midnight here and i'm tired, i'll have another look tomorrow to try and see how what i've done may be different to how ec2 does it and will ask you if i need to [13:51] wallyworld__: ok, np [13:51] thanks for asking [13:52] wallyworld__: FWIW in the ec2 provider, the public storage is implemented with exactly the same type as the private storage [13:52] i do that too [13:52] the public openstack storage uses a client connection that doesn't fo any authentication [13:52] wallyworld__: it's probably down to the way the container object is initialised [13:53] the public storage requires that the container be created with the correct acl to allow world readable without auth being required [13:53] and so the reading bit works fine [13:54] ie someone who is authenticated can upload stuff to the container and then anyone can read it, autjenticated or not [13:55] however, using the public container to try and write something eg fake tools in the tests, fails because no auth is used since the public container uses a non authenticating client [13:55] s/public container/public storage [13:56] wallyworld__: is the issue that you *can't* use authentication when connecting to a container that doesn't require it? [13:56] i could, but the public storage is initialised with a non authenticating client [13:56] openstack clients store the auth token when authentication occurs [13:56] wallyworld__: could you initialise it with an authenticating client? [13:56] and then put that token in each request [13:57] yes, but that would require authentication which the public containers must not require? [13:57] wallyworld__: i'm presuming the crux is in this code: http://paste.ubuntu.com/1534329/ [13:57] the non public storage in the openstack provider does indeed use an authenticating client [13:58] yes [13:58] the public storage is initialised with a client which does not authenticate, so no credentials are used [13:59] used or required [13:59] wallyworld__: what is it about the publicStorageUnlocked initialisation that isn't authenticating? [13:59] publicBucketClient [13:59] wallyworld__: i see two occurrences of e.client(ecfg, authMethodCfg) [13:59] wallyworld__: ah, i may be looking at an old branch though [13:59] let me check the code [14:00] func (e *environ) publicClient(ecfg *environConfig) client.Client { [14:00] return client.NewPublicClient(ecfg.publicBucketURL(), nil) [14:00] } [14:00] around line 196 [14:01] the public storage instance uses a goose client that is non authenticating as constructed above [14:01] wallyworld__: ah, i wasn't in sync [14:01] public bucker url above is like the public region for ec2 [14:02] so a goose client can be created with credentials, or not [14:02] if credentials are provided, it authenticates [14:03] wallyworld__: so if you provide credentials for a container that doesn't require them, it fails? [14:03] there are separate constructors for each type - you cannot supply credentials for a public client [14:04] if use curl to poke a container which is publicly readable but use credentials, i'm not sure what will happwn [14:04] i think i read that there is a but in that area and it will not work [14:05] bug not but [14:05] wallyworld__: that's what i was wondering [14:05] i definitely read that there is a bug in that area, but am uncertain if it is fixed. i will need to test [14:06] wallyworld__: it seems a bit wrong that it can't ignore credentials when they're not required [14:06] indeed [14:07] wallyworld__: i wonder if it would be possible to work around that bug [14:07] wallyworld__: for instance, by detecting an error code that signifies the bug [14:07] not sure, but it does seem the idea of a non authenticating client needs to be rethought [14:08] wallyworld__: goose/client could certainly do with some API docs - i can't see how it's meant to work... [14:08] rogpeppe, https://codereview.appspot.com/7058073 has changed a bit since I merged trunk in and might benefit from a second glance; TheMue, you've also started to review that one [14:08] fwereade_: Will take a look. [14:09] TheMue, tyvm [14:09] rogpeppe: yes, it probably does need more doc [14:09] it's very openstack specific [14:10] to understand it you need you to know about how openstack works [14:10] wallyworld__: if it's openstack specific, perhaps it should be hidden behind the swift, etc APIs [14:10] wallyworld__: rather than requiring clients to interact with it [14:11] the goose client is used by the goose swift code to provide the underlying transport/connectivity [14:11] a client needs to be created with credentials, but after that, it's opaque [14:12] it just needs to be passed to a swift instance so that it can be used [14:12] wallyworld__: perhaps the goose swift code should provide a function that encapsulates that, so the type isn't externally visible. [14:12] is there a command to show the complete effective environ config as seen by juju? [14:12] the same client is used by the nova and glance stuff too [14:12] after defaults, etc. are applied? [14:13] dimitern: you can call AllAttrs [14:14] wallyworld__: hmm, yes, i see the issue [14:14] rogpeppe: on what? [14:14] dimitern: config.Config [14:14] dimitern: what's your context? [14:15] rogpeppe: I want to dump the config after validation, before any cmd like bootstrap is executed [14:15] wallyworld__: if NewPublicClient and NewClient were merged, i'm not sure there would be a good reason for exposing Client. [14:16] rogpeppe: isn't there something like juju get --all ? [14:16] rogpeppe: the same client instance is shared by nova and swift instances [14:16] dimitern: ah, from the command line [14:16] rogpeppe: yeah, that seems easiest [14:16] ah, no it's not [14:17] a new client instance is created for each nova, swift instance [14:18] rogpeppe: i sort of think of a goose client as like a tcp socket - you may need to create one and then give it to things to use on your behalf but you don't need to know how a socket works [14:18] but i guess you are saying each thing should create it's own socket [14:19] wallyworld__: well, i think that might possibly make for a more straightforward API, but i'd need to think about it [14:20] wallyworld__: in general though, whether openstack-specific or not, any exported API should be fully documented [14:20] the thing with merging NewClient and NewPublicClient is that go lacks default method params etc, so it gets messy [14:20] yeah, agree with doco - we've just run way short on time [14:21] wallyworld__: i don't think that's necessarily a problem. NewClient has quite a few args anyway; it could easily be a struct. [14:21] yes. i used that approach in some of the client internals [14:21] wallyworld__: type ClientParams struct {Creds ...; AuthMethod ...; Logger; etc} === TheMue_ is now known as TheMue [14:22] wallyworld__: that's the go way to do default method params. works pretty well usually. [14:22] yeah, i gotta get used to that. hard to forget all that python [14:22] it does seem to add overhead though [14:23] compared with just using default method or kw args [14:23] and that overhead is incurred for each and every call [14:23] wallyworld__: runtime overhead? [14:23] since the struct needs to be created each time [14:23] typing/verbosity overhead [14:24] code bloat [14:24] wallyworld__: there are advantages too though - you can create an object encapsulating the params and pass it around and manipulate it easily [14:24] rogpeppe, TheMue, niemeyer: can anyone remember why we prevent SetExposed and ClearExposed when a service is Dying? [14:25] rogpeppe: swings and roundabouts as with anything :-) [14:25] fwereade_: it was a fairly arbitrary decision AFAIR [14:25] rogpeppe: so I guess there isn't a way to dump the config from the command line? [14:25] dimitern: you could write a program to do it in 3 minutes [14:26] rogpeppe: thanks for discussion, way past my bedtime, i'll look again tomorrow [14:26] wallyworld__: cheers! [14:26] rogpeppe: ok, so I need to dump AllAttrs from environ? [14:27] dimitern: i think you'd print the result of environs.NewFromName("").Config().AllAttrs() [14:27] rogpeppe: I'll try that, 10x [14:27] rogpeppe, I don't think I can justify disallowing ClearExposed; SetExposed is more interesting, but I'm inclined to say it should be allowed for symetry's sake [14:27] dimitern: assuming you're using the standard environments.yaml [14:28] fwereade_: i'm not sure the operations need to be symmetrical [14:28] fwereade_: we often allow closing-down operations when dying, but not adding operations [14:28] rogpeppe, yeah, true [14:29] fwereade_: mind you, i always thought there should only be one call: SetExposed(bool)... [14:30] rogpeppe, yeah, I can sympathize there [14:30] fwereade_: I think either way is fine [14:31] rogpeppe: Yeah, I recall that :) [14:31] The reasoning, ironically, is precisely for symmetry.. [14:31] Other flags require more than a bool [14:31] But I digress [14:31] and will have lunch, in fact.. biab [14:32] fwereade_: just looking at https://codereview.appspot.com/7058073 - quite a bit has changed, but i don't see my comments addressed [14:33] rogpeppe, oh, hell, I got completely distracted by the manual merge [14:33] fwereade_: np [14:33] rogpeppe, I don't have any objections to any of them though [14:33] fwereade_: cool [14:40] fwereade_: you've got a review [14:40] TheMue, ty [14:45] fwereade_: ping [14:45] rogpeppe, pong [14:46] fwereade_: i'm starting to think that all public Dying methods are crackful (other than Tomb's, of course). thoughts? [14:47] fwereade_: the fact that something is dying is really an internal detail - what an external thing should care about is dead or not dead IMO [14:47] rogpeppe, hmm -- I can't think of many places they're used but I'm not yet ready to commit to such a statement myself [14:47] rogpeppe, wait, you mean on state entities? or on watchery things? [14:47] fwereade_: so for instance, the only place you use Uniter.Dying, you could (should, i think) use u.tomb.Dying instead [14:47] fwereade_: both [14:48] rogpeppe, when is there a Dying method that isn't `return x.tomb.Dying()`? [14:48] fwereade_: here: http://paste.ubuntu.com/1534428/ [14:49] fwereade_: oops, those are calls [14:49] fwereade_: i don't think anything should expose its internal tomb's dying state [14:49] fwereade_: what's it useful for? [14:50] fwereade_: it says "this object is starting to shut down", but that's not useful info when interacting with the object. [14:50] fwereade_: i think all those Dying methods should be Dead methods that return x.tomb.Dead() === slank_away is now known as slank [14:53] fwereade_: for example, the comment here is just wrong: [14:53] // Dying returns a channel that signals a Firewaller exit. [14:53] func (fw *Firewaller) Dying() <-chan struct{} { [14:54] fwereade_: it doesn't signal a firewaller exit, but just that at some time in the future the firewaller hopefully will exit. [15:32] lunch [16:19] fwereade_: what was the last thing you saw me say before i said "lunch" ? [16:19] rogpeppe, fwereade_: it says "this object is starting to shut down", but that's not useful info when interacting with the object. [16:20] ah [16:20] [14:50:46] fwereade_: i think all those Dying methods should be Dead methods that return x.tomb.Dead() [16:20] [14:53:40] fwereade_: for example, the comment here is just wrong: [16:20] [14:53:40] // Dying returns a channel that signals a Firewaller exit. [16:20] [14:53:40] func (fw *Firewaller) Dying() <-chan struct{} { [16:20] [14:54:06] fwereade_: it doesn't signal a firewaller exit, but just that at some time in the future the firewaller hopefully will exit. [16:21] rogpeppe, that also sounds sane, probably, where there are places it's actually used [16:21] rogpeppe, how common is that? [16:21] fwereade_: there's one use, in a test. [16:22] rogpeppe, heh, for something that doesn't just have a .Wait()? [16:22] fwereade_: no, so that we can time out a wait [16:22] fwereade_: all the other uses of Dying other than on a tomb are in uniter, and those could all be u.tomb.Dying [16:23] rogpeppe, consider me +1 then [16:23] rogpeppe, not sure why I ever had u.Dying() [16:23] fwereade_: i tell a lie, it's never used [16:24] rogpeppe, cool [16:24] fwereade_: i thought f.Dying was on a firewaller (not looking at the file) but it was on a uniter.filter. [16:24] rogpeppe, ah, yeah [16:25] niemeyer: does this sound reasonable to you: change all public Dying methods in juju-core into Dead methods? [16:26] rogpeppe: Huh!? [16:26] niemeyer: there's no use for Dying, but there is a use for Dead [16:26] niemeyer: dying in an internal state [16:26] s/in an/is an/ [16:26] rogpeppe: Sorry.. quite out of context here [16:27] rogpeppe: The quick answer is no, it doesn't sound reasonable without further detail [16:27] niemeyer: that's fine. let me try to explain. [16:27] niemeyer: 1) nothing ever uses a Dying method currently (other than Tomb.Dying) [16:28] niemeyer: 2) ... except for one case in a test where it should be using Dead. [16:28] niemeyer: 3) i'd like to use a Dead method (on api.Server) [16:29] niemeyer: 4) i believe that "dying" cannot signify anything useful to a user of an object [16:29] niemeyer: because it's really a transitional state [16:29] rogpeppe: What's the test that is broken? That may be an interesting case to look at [16:30] rogpeppe: Not really.. Dying isn't transitional [16:30] niemeyer: here's the code: http://paste.ubuntu.com/1534705/ [16:30] niemeyer: the bug is that it can wait 50ms for dying to happen, but then the transition to dead might take a lot longer [16:30] rogpeppe: tomb.Dying and other similar channels fire once the thing is dying, and always from then on [16:31] niemeyer: i mean that dying is part of the transition to dead [16:31] niemeyer: i realise that the channel state is persistent [16:31] rogpeppe: The point is that Dying, the method you're suggesting we remove, is not transitional.. it has well defined behavior that is fire-once [16:31] niemeyer: can you think of a good use case for Dying? [16:31] niemeyer: as a publicly visible method [16:31] rogpeppe: Given the dozens of cases we have with tomb.Dying, of course.. [16:32] niemeyer: i'm not including tomb.Dying. [16:32] niemeyer: that's fine as is [16:32] niemeyer: it's the tomb's raison d'etre [16:32] rogpeppe: Not really.. Tomb is only useful with the full behavioral pack it has [16:32] rogpeppe: Including Dead, Kill, etc [16:33] niemeyer: but a tomb is an implementation detail of an object, which is why we have it as a non-exported field, rather than embedding it. [16:33] rogpeppe: Wait [16:33] niemeyer: ok, true [16:33] rogpeppe: So I'm looking at the paste [16:33] rogpeppe: It's not clear how that proves Dying is bad [16:33] niemeyer: "dead not detected" [16:34] niemeyer: it's waiting for the object to die. but Dying does not signify that. it signifies that it will die (hopefully soon) [16:34] rogpeppe: Sure.. it's a bit like saying Sleep is back because it blocks [16:34] s/back/bad [16:35] niemeyer: why would we ever want to wait until an object *starts* to clean itself up? [16:35] niemeyer: i think that we always want to wait until it *has* cleaned itself up [16:35] rogpeppe: Always when? [16:35] niemeyer: (unless we're part of the object itself, and need to participate in the cleaning-up process) [16:35] rogpeppe: It feels like we're trying to guess stuff up unnecessarily [16:36] rogpeppe: All I see is one test poorly written [16:36] rogpeppe: That doesn't say much about Dying [16:36] niemeyer: that's the *only* place that Dying is used (other than some places in uniter that could use the tomb Dying method) [16:36] niemeyer: i mean Dying-other-than-tomb.Dying there [16:37] rogpeppe: Erm [16:37] niemeyer: the only place that one of the Dying methods defined in juju-core is used [16:37] niemeyer: to be more accurate [16:38] rogpeppe: UnitDying? [16:39] niemeyer, UnitDying is justa broadcast channel that's closed when the unit's Life is Dying -- has nothing to do with tombs [16:39] niemeyer: that's different. it's not about the object itself [16:40] niemeyer: why would we ever want to wait until an object *starts* to clean itself up? [16:40] [16:35:40] niemeyer: (unless we're part of the object itself, and need to participate in the cleaning-up process) [16:40] rogpeppe: UnitDying is not part of the object itself [16:40] rogpeppe: Someone else watches it [16:40] in this case, the uniter could arguably be considered part of the unit "objecT" [16:41] niemeyer: i would not suggest changing UnitDying to UnitDead [16:41] rogpeppe: Me neither.. Dying means what it means, and seems to be fine everywhere we've used it [16:41] rogpeppe: If you use Dying when you mean Dead, it won't fly so well, though [16:41] rogpeppe: Same thing if you use Dead when you mean Dying [16:41] niemeyer: which is... in one place, wrongly. [16:42] rogpeppe: In one place if you special case-out every other place [16:42] niemeyer: i don't see why we expose Dying on Firewaller, for example [16:42] niemeyer: or Provisioner or Uniter [16:43] niemeyer: or uniter.filter [16:43] niemeyer: all those places would be much better served with a Dead method AFAICS [16:44] rogpeppe: That's an easier to verify assertion [16:44] rogpeppe: rather than a witch-hunting against Dying methods [16:44] * niemeyer looks [16:44] niemeyer: those are the only Dying methods we define [16:44] rogpeppe, don't all tasks have Wait() though anyway? [16:44] rogpeppe: Heh [16:44] fwereade_: yes. but Dying is nicer than starting a new goroutine just to call wait and send on a channel [16:45] fwereade_: when we've got that channel available anyway [16:45] oops! [16:45] fwereade_: Dead! [16:45] rogpeppe, haha :) [16:45] rogpeppe, I'll be guided by you in that, you're deeply involved with the client ATM [16:46] rogpeppe, AIUI that's what the client does at the moment, right? [16:46] fwereade_: which client? [16:46] rogpeppe, jujud [16:47] rogpeppe: So firewaller.Dying is never used? [16:47] niemeyer: indeed [16:47] rogpeppe: Easy one.. just kill it [16:47] niemeyer: +1 [16:47] Huh [16:47] niemeyer: and the other never-used Dying methods too? [16:47] Why are we even discussing this? [16:47] niemeyer: because i'm just about to create a Dead method [16:48] niemeyer: and was wondering about the precedent. [16:48] rogpeppe: Why do we need one? [16:48] rogpeppe: That's different [16:48] rogpeppe: Removing an unused method is a pretty easy choice [16:48] rogpeppe: No matter its name :) [16:48] niemeyer: we don't *need* one - we can always do go func() {c <- obj.Wait()}; but a Dead method seems nicer, since the channel is available anyway [16:49] rogpeppe: Sure, if you need Dead just add it [16:49] niemeyer: ok, cool [16:50] rogpeppe: That's a different conversation than "let's kill all Dying methods and replace them by Dead because that's always what we need", if you see what I mean [16:50] niemeyer: i guess so. i had hypothetical use cases in mind, not real ones :-) [16:50] rogpeppe: There's no long term decision, or convention being established.. just add that one-liner you need and let's be happy :) [16:51] niemeyer: and i still think that an externally visible Dying method that just returns obj.tomb.Dying() is almost certainly going to be a mistake [17:05] hmm, weird codereview behaviour i haven't seen before: https://codereview.appspot.com/7101059/ [17:06] fwereade_: do you see no diffs at all? [17:06] the diffs in launchpad look fine [17:13] rogpeppe: No issue exists with that id (7101059) [17:13] rogpeppe: That's the message I get [17:13] niemeyer: i've just deleted it, trying again. [17:13] niemeyer: but the next one has the same problem too [17:13] niemeyer: one mo [17:13] rogpeppe: FWIW, I just got a weird behavior while using it too.. had to reload the CL *several* times to see comments I had just entered [17:13] niemeyer: https://codereview.appspot.com/7106052 [17:14] rogpeppe: And my comments disappeared again! [17:14] rogpeppe: There's something funky going on in their side [17:14] niemeyer: i guess you could review it by looking at the launchpad page [17:15] niemeyer: there's nothing complicated going on [17:20] rogpeppe: It's not okay for lbox/codereview to be broken like that [17:21] niemeyer: i've never seen this particular failure-mode before [17:21] niemeyer: i imagine there's a data warehouse gone down somewhere :-) [17:25] rogpeppe: The changes to u.Dying => u.tomb.Dying seem undue [17:26] rogpeppe: u.Dying seems perfectly okay there [17:26] niemeyer: why would something external to uniter find any use from using the Dying method? [17:26] niemeyer: the dying state is surely something internal to the implementation of Uniter? [17:27] niemeyer: as it is in all our other objects [17:27] fwereade_: what do you think? [17:28] rogpeppe: Maybe [17:28] rogpeppe: none of our other objects tend to touch their tombs from external functions, though [17:28] niemeyer: neither does uniter [17:29] rogpeppe: See modes.go [17:29] rogpeppe: I don't care enough, though [17:30] niemeyer: modes.go is still internal to the package, although it's true it defines functions not methods [17:30] niemeyer: but that's not unprecedented. in jujud, i pass tombs around to other functions. [17:30] Whatever William says is fine by me [17:30] rogpeppe: Yes, you do.. [17:30] rogpeppe: Other functions don't grab private tombs, though [17:31] niemeyer: the modes.go functions use other private Uniter fields too [17:31] rogpeppe: I don't care, though.. it feels a bit of a red-herring change, and a bike-sheddy discussion [17:31] niemeyer: i don't think the tomb is much different [17:32] rogpeppe: LGTM and let's focus on something more valuable [17:32] niemeyer: i didn't plan on spending more than 5 minutes on this change! [17:32] rogpeppe: Zero would be a better number [17:33] looks like I'm going to be without power for about 2 hours here this afternoon [17:34] I'll be around though, so if you you need to contact me I will be checking e-mail periodically [17:34] and will be available by phone [17:34] rogpeppe: Regarding the yaml vs. json change, we can use yaml, and fix goyaml later to use !binary, that encodes as base64 by itself [17:35] niemeyer: i think if we use yaml we'll exceed the 16K cloudinit limit [17:35] rogpeppe: "The server encountered an error and could not complete your request." [17:35] rogpeppe: codereview is busted [17:35] niemeyer: seems like it [17:35] rogpeppe: I don't understand [17:35]