[00:01] <bigjools> davecheney: does the core team have any plans to set up CI for juju?
[00:15] <davecheney> bigjools: yes, we want to set that up
[00:15] <davecheney> but we're still stuck on the dependency management problem
[00:15] <davecheney> at the moment tarmac and lb build recipies are the best we have
[00:15] <bigjools> davecheney: right - nobody replied to my email suggesting a way of handling that.
[00:16] <bigjools> davecheney: also at the sprint here I want to move gwacl over to the same bot that lands the core
[00:21] <davecheney> bigjools: ok, cool
[00:22] <davecheney> sorry, stil on da phone
[00:22] <bigjools> np
[00:23]  * bigjools picked a hotel for everyone. It's not too late to move it to a grotty backpackers hostel.
[02:00] <davecheney> axw: did you want to try for a 1.13.1 release tomorrow or friday to pick up debug-hooks ?
[02:01] <davecheney> when i say 'you', i mean 'me', except when it comes to writing the release notes, in which case i mean 'you' :)
[02:01] <axw> davecheney: I'm happy to write the release notes. I understand people are waiting for it, so I guess now's as good a time as any
[02:02] <axw> I'll write something up after finishing reviews
[02:03] <davecheney> axw: it can be as simple as 'debug-hooks' now works
[02:03] <davecheney> if there are any operatoinal diffrences just call them out
[02:03] <davecheney> but it sort of works as expected right ?
[02:03] <axw> yeah there shouldn't be any differences
[02:03] <axw> I'll read over the docs again to make sure
[02:06] <davecheney> axw: https://docs.google.com/a/canonical.com/document/d/1XPzmRAM3W7oYYmCGVYB5nR1DZyB16t8qPF68xxmWZho/edit#
[02:07] <axw> davecheney: ta
[02:10] <axw> davecheney: yeah, there are no operational differences between pyjuju and juju-core implementations
[02:10] <axw> so I guess your one liner is good enough
[02:11] <davecheney> axw: marco and I are going to use it release notes or not
[02:11] <davecheney> :)
[02:11] <axw> use what?
[02:11] <davecheney> we're doing some training next week, north of australia
[02:12] <davecheney> so debug hooks would be usefu;
[02:12] <axw> ah
[02:12] <axw> cool :)
[02:12] <davecheney> it is one of the last things keeping people on pyjuju
[02:12] <davecheney> that and the argument about provider specific constraint names
[03:21] <axw> davecheney: has anyone written anything substantial about what's expected of the null/manual provider?
[03:22] <axw> e.g. what is it meant to do when a request for a new instance is made?
[03:44] <davecheney> axw: willimam would have to be the one to answer that
[03:45] <davecheney> it sounds more like a pipe dream
[03:45] <axw> davecheney: ok, ta
[03:45] <axw> you don't think it's feasible?
[03:45] <davecheney> but i guess it operates like a provider where someone has called 'add-machine' to prime th environment
[03:45] <davecheney> axw i haven't see a spec, just a lot of hyperventilation
[03:45] <davecheney> so i'll reserve judgement
[03:45] <axw> right
[03:55] <jtv> Hi wallyworld_ -- could I quiz you a bit more about simplestreams?
[03:55] <wallyworld_> jtv: sure, give me a couple of minutes
[03:55]  * jtv gives wallyworld a couple of minutes
[04:11] <wallyworld_> jtv: z'up
[04:14] <jtv> Hi wallyworld_.  I was wondering about the daily vs. release simplestreams.
[04:15] <jtv> How should we arrange configuration for this?
[04:15] <wallyworld_> what were you wondering?
[04:15] <jtv> Here's what:
[04:15] <wallyworld_> i just use the release ones
[04:15] <jtv> Default would get the base URLs for both the release and the daily simplestreams.
[04:15] <wallyworld_> i ignore the dailies
[04:15] <jtv> The Stream would be configurable,
[04:16] <jtv> and if desired, we could allow configuration to override the base URLs -- but hopefully nobody would really use that.
[04:16] <wallyworld_> for ec2 etc, the base url is the same for both
[04:16] <wallyworld_> as far as i remember
[04:16] <wallyworld_> the difference is in the product ids inside the index
[04:16] <jtv> It all depends on how far down that tree we navigate, doesn't it?  Lemme check
[04:17] <jtv> https://cloud-images.ubuntu.com/releases/streams/v1/
[04:17] <wallyworld_> my memory might be wrong
[04:17] <jtv> https://cloud-images.ubuntu.com/daily/streams/v1/
[04:17] <jtv> I think those need to be separate base URLs.
[04:18] <wallyworld_> ah, that looks lke it may have changed since i last looked at it
[04:18] <wallyworld_> yes, in that case, the base urls need to be different
[04:18] <jtv> But I was thinking: if we get to select the stream, we might as well include both URLs by default.
[04:18] <wallyworld_> i haven't put the base url into config yet
[04:19] <jtv> Because we want the config to be flexible, but also, simple.
[04:19] <jtv> That explains why searching the providers for "daily" didn't get me anything.  :)
[04:19] <wallyworld_> i think including both urls will work
[04:19] <wallyworld_> but i think we may also need a user override for base url
[04:20] <jtv> It should -- everything is designed for multiple base URLs.  I'd be happy to leave them hard-coded for now, and make the stream configurable.
[04:20] <wallyworld_> sure
[04:21] <jtv> IIRC we get to pick 1 stream in our image constraint.  I wonder if we should expose it in the config that way, or let the user specify multiple streams in order of preference and loop over calls to imagemetadata.
[04:21] <wallyworld_> i think using one stream only is easier to understand
[04:22] <jtv> True... and having an order of preference would be something that belonged in imagemetadata anyway.
[04:22] <jtv> So I'll go with a single config item then: "stream."  It'll default to the releases stream.
[04:22] <wallyworld_> sounds good
[04:22] <jtv> (I think that stream is called "released" but its index is called "releases" or something like that)
[04:23] <wallyworld_> i'm about to propose a pretty big (in terms of diff) change to simplestreams stuff - to have a generic part and a type specific part
[04:23] <jtv> Ah yes, the stream is called "released" but the base URL says "releases".
[04:23] <jtv> Thanks!
[04:23] <wallyworld_> so we can have image metadata and easily add tools metadata
[04:23] <wallyworld_> most of the code is framework, the image andtools bits just define some type specific artifacts
[04:26] <jtv> wallyworld_: actually it looks as if maybe imagemetadata.DefaultBaseURL should be an array: [released, daily].
[04:27] <jtv> But absent this same streams configuration in the other providers, that would suddenly make them use daily images.  :/
[04:28] <wallyworld_> the daily one could come second? and isn't daily used in the product id? no no matches would occur?
[05:27] <jtv> wallyworld_: I got called away...  Yes, you're right, I think the way imagemetadata works, listing the releases URL first would mean that providers would still get the released images by default.
[05:29] <jtv> So that might be worth changing globally.  Then all providers would know of both the released and the daily streams out of the box, with released being the default, and it'd be easy to add configuration options for "daily."
[05:29] <jtv> In fact I don't see why any of this should be provider-specific.
[06:02] <wallyworld_> jtv: it is provider specific because different providers has their own way of giving out metadata. only CPC's use the default base urls
[06:03] <wallyworld_> eg canonistack defines a keystone endpoint
[06:03] <wallyworld_> and we want the default urls to be used last so users can override them
[06:05] <jtv> I see.
[06:05] <jtv> Still, selecting a stream should be pretty standard, no?
=== tasdomas_afk is now known as tasdomas
[06:29] <wallyworld_> yes
=== tasdomas is now known as tasdomas_afk
[07:30] <noodles775> Thanks for the review axw. Anyone around who can do a second review for a simple branch - adding environment name to the status output: https://codereview.appspot.com/12535043
[07:30] <axw> nps
[07:33] <sidnei> thumper: here's a fun one if you want to take a look: http://paste.ubuntu.com/5957994/ was midway through bringing a unit up when my battery died, now it's stuck in a loop trying to recover from what seems to be an unrecoverable state
[07:38] <sidnei> ok, blowing away state/bundles and state/deployer made it get unstuck
=== tasdomas_afk is now known as tasdomas
[07:59] <rogpeppe> mornin' all
[08:00] <rogpeppe> anyone up for giving me another review of this? https://codereview.appspot.com/12473043/
[08:01] <rogpeppe> and i'm after any reviews of this: https://codereview.appspot.com/12551043/
[08:04] <wallyworld_> rogpeppe: hi, i'm about to go play soccer. i re-proposed some changes to the branch. also, don't forget the plugin params branch :-) i'll be back in a few hours
[08:04] <rogpeppe> wallyworld_: will do. have fun!
[08:05] <wallyworld_> thanks!
[08:37] <rogpeppe> mgz: could you have a quick glance at https://codereview.appspot.com/12361043/ please, just to go over my additions to the CONTRIBUTIONS file?
[08:37] <rogpeppe> dimitern: you've been waiting for this a whle: https://codereview.appspot.com/12551043/
[08:38] <dimitern> rogpeppe: cheers!
[08:41] <dimitern> rogpeppe: ian sent me an email that he's going to miss today and tomorrow's standup due to soccer
[08:41] <rogpeppe> dimitern: ok
[08:41] <dimitern> rogpeppe: did you manage to chat with him yesterday?
[08:41] <rogpeppe> dimitern: no
[08:42] <dimitern> rogpeppe: well, it's friday then
[08:42] <rogpeppe> dimitern: i
[08:42] <rogpeppe> dimitern: i might be able to catch him today after soccer
[08:43] <dimitern> rogpeppe: ah, ok
[08:53] <axw> rvba: ping?
[08:53] <rvba> Hi axw
[08:54] <axw> rvba: hi. regarding the NetworkConfigurationSet thing...
[08:54] <axw> NewNetworkConfigurationSet takes a slice, which you're passing in as nil
[08:54] <axw> it then takes the address
[08:54] <axw> so it's never going to be a nil pointer
[08:56] <axw> rvba: nto sure if this is really a problem or not, I'm just going by what the gwacl comment says
[08:56] <rvba> axw: right, but that's not a problem because NewNetworkConfigurationSet creates a configuration of type "NetworkConfiguration" so we don't need the "subnetNames" element to be omitted.
[08:58] <rvba> axw: NewLinuxProvisioningConfigurationSet creates the same type of XML element but with a type "LinuxProvisioningConfiguration".  In this case, the "subnetNames" element *must* be omitted and we use the nil pointer trick.
[08:58] <axw> gotcha
[08:58] <axw> sorry!
[08:58] <axw> I will LGTM now
[08:58] <rvba> Sorry *I* wasn't very clear before :)
[08:58] <rvba> Ta.
[09:01] <dimitern> rogpeppe: I particularly like the "oneLife" method
[09:01] <dimitern> :)
[09:05] <dimitern> rogpeppe: I don't think you merged my trunk changes into your branch though
[09:05] <rogpeppe> dimitern: no, good point - i'll do that
[09:13] <dimitern> rogpeppe: reviewed
[09:13] <rogpeppe> dimitern: ta!
[09:16] <rogpeppe> dimitern: i removed the test cases because they're a little more awkward to implement with the shared fakeState and i didn't think they added anything significant to the test coverage, but actually i think you're right; i'll reinstate them
[09:17] <dimitern> rogpeppe: thanks!
[09:18] <dimitern> rogpeppe: I suspected it was a bit awkward to implement them, but they still test a valid case, which shows we handle state errors correctly
[09:21] <noodles775> dimitern: thanks for the second review. I've done rv-submit (ie. just adds reviewers to description for me), could you please manually mark the MP approved? https://code.launchpad.net/~michael.nelson/juju-core/1208505-status-env/+merge/178766
[09:21] <sidnei> https://bugs.launchpad.net/juju-core/+bug/1209112
[09:21] <_mup_> Bug #1209112: deployer stuck in unrecoverable loop after power loss <juju-core:New> <https://launchpad.net/bugs/1209112>
[09:27] <dimitern> sidnei: sorry, I don't really understand what was the issue there
[09:27] <sidnei> dimitern: the zip file in the state dir was corrupted
[09:27] <sidnei> dimitern: or at least that's my understanding
[09:27] <dimitern> sidnei: how did that happen?
[09:27] <sidnei> dimitern: local deployer, ran out of battery in my laptop
[09:29] <dimitern> sidnei: but it seems the issue was with the uniter not downloading the complete zip file, not the deployer
[09:30] <sidnei> dimitern: i probably got them confused, yes
[09:32] <dimitern> sidnei: I edited the description, but otherwise it seems like a genuine uniter issue
[09:32] <dimitern> sidnei: thanks for the report
[09:33] <sidnei> np!
[09:44] <dimitern> rogpeppe: unit agents are now deployed with APIInfo, right?
[09:45] <rogpeppe> dimitern: yes
[09:45] <rogpeppe> dimitern: they even connect to the API now
[09:45] <dimitern> rogpeppe: great, so we have half of the blueprint implemented now :)
[09:52] <noodles775> axw: Thanks for the approve. The mp failed with a test failure... all tests pass for me locally, so I tried re-merging devel and running that test, it still passes: http://paste.ubuntu.com/5958302/ . Any ideas?
[09:54] <axw> noodles775: not sure what happened there, it looks unrelated tho.  Perhaps a race condition in another test. I'll reapprove
[10:06] <dimitern> rogpeppe: I though you fixed that http://paste.ubuntu.com/5958349/
=== axw is now known as axw_
[10:09] <axw_> noodles775: it worked this time
[10:09] <rogpeppe> dimitern: i'm still waiting for a second review on the branch
[10:09] <dimitern> rogpeppe: no, I meant TestMachineWatch
[10:09] <dimitern> axw_: it's intermittent
[10:09] <rogpeppe> dimitern: that's what i'm talking about
[10:09] <rogpeppe> dimitern: https://codereview.appspot.com/12352044/
[10:09] <dimitern> rogpeppe: ah, I see
[10:10] <dimitern> axw_: can you review that please? ^^
[10:10] <axw_> yup, may be beyond me tho
[10:10] <rogpeppe> axw_: a good opportunity to delve into the details of the state/watcher code :-)
[10:11] <axw_> :)
[10:11] <dimitern> axw_: take a look at State.Sync and State.StartSync - that's the crux of the problem
[10:11] <axw_> dimitern: thanks
[10:11] <noodles775> axw_: Thanks - merged that time.
[10:12] <axw_> noodles775: nps
[10:12] <rogpeppe> axw_: and note the way that the watcher code uses time.After(0) when it gets a sync request. that's part of the source of this problem, i think.
[10:13] <dimitern> noodles775: btw you might want to get rid of cobzr and use bzr instead - cobzr was created when bzr didn't support some of the features, like lightweight checkouts
[10:18] <noodles775> dimitern: really? hrm - we should update the CONTRIBUTING doc. bzr has supported lightweight checkouts for a long time (I mean, I wrote this tutorial using them in 2011... https://micknelson.wordpress.com/2011/05/19/sharing-your-development-environment-across-branches/ )
[10:18] <dimitern> noodles775: yeah, that part in CONTRIBUTING needs to be updated
[10:19] <noodles775> dimitern: Cool. I was wondering whether it'd be worth adding a make target that does most of what CONTRIBUTING says (or should say). If you guys can let me know what the proper way is atm, I can update it and/or add the target.
[10:20] <noodles775> For eg, currently I'm installing go from godeb (1.1.1), and downloading the mongodb executables and putting them in in $GOPATH/bin, but is the PPA preferred now?
[10:21] <dimitern> noodles775: ppa's version of mongodb is not yet usable I think - at least I couldn't make it work couple of weeks ago when I tried last
[10:25] <axw_> rogpeppe: it looks sound, but do you mind if I go over it more closely in the morning when I'm more awake?
[10:25] <axw_> haven't had much sleep :)
[10:28] <noodles775> dimitern: what about go - is godeb install 1.1.1 preferred for a dev env, or a PPA?
[10:29] <dimitern> noodles775: we all should be using the golang version from ppa:james-page/golang-backports
[10:29] <dimitern> noodles775: it's 1.1.1 + some post-release fixes, that will be like that until we backport it to the archive
[10:30] <noodles775> k
[10:31] <jamespage> dimitern, actually that reminds me - mgz - I pushed an up-to-date as of yesterday morning copy of golang 1.1.1 into https://launchpad.net/~james-page/+archive/golang-backports
[10:31] <jamespage> do you want to sync that to your official ~juju golang PPA?
[10:31] <jamespage> noodles775, dimitern: this PPA would be better - https://launchpad.net/~juju/+archive/golang
[10:32] <dimitern> jamespage: ah, ok, good to know
[10:32]  * dimitern bbiab
[10:32] <jamespage> thats what we are using todo all of the juju-core backport builds
[10:32] <noodles775> Great - thanks jamespage
[10:32] <jamespage> davecheney, still awake?
[10:36] <mgz> jamespage: yup, we should
[10:46] <mgz> copied acriss
[10:46] <mgz> *acroo
[10:46] <mgz> ...
[10:46] <mgz> across
[10:47] <mgz> removed the saucy one as that doesn't need backporting
=== marcoceppi_ is now known as marcoceppi
[11:31] <dimitern> rogpeppe, mgz, natefinch: standup
[11:35] <rogpeppe> mgz: you still there?
[11:36] <mgz> gsh!
[11:36] <mgz> how long was I talking for when g+ decided to log me out
[11:36] <rogpeppe> mgz: not v long
[11:36] <mgz> >_<
[12:18] <natefinch> rogpeppe, dimitern, mgz: Do we avoid concrete error types on purpose? I've noticed a lot of places in the code where we do string comparisons on error messages, and it makes me antsy, because I always consider error messages to be something you shouldn't count on staying the same.
[12:18] <rogpeppe> natefinch: i think we only do that in tests
[12:19] <natefinch> rogpeppe: ok, that's fair
[12:21] <rogpeppe> dimitern: reproposed, with all your requested fixes made, i think. i also added the same error-returning test to the other places that didn't do it. https://codereview.appspot.com/12551043/
[12:21] <dimitern> rogpeppe: great, thanks! will take a look in a bit
[12:43] <rogpeppe> i wondered why the network was down. turned out that Carmen had accidentally unplugged the ethernet switch when cleaning
[12:47] <dimitern> :)
[12:56] <dimitern> natefinch, rogpeppe: reviewed
[12:56] <natefinch> dimitern:  thanks!
[12:56] <rogpeppe> dimitern: thanks!
[12:57] <rogpeppe> dimitern: the TODO now lives in the FindEntity method
[12:57] <dimitern> rogpeppe: ah, ok I missed that then
[13:04]  * rogpeppe goes for lunch
[13:40] <frankban> could anyone please take a look at https://codereview.appspot.com/12522043? thanks!
[13:45] <dimitern> frankban: i will
[13:48] <frankban> dimitern: thank you
[13:58] <dimitern> frankban: reviewed
[14:00] <frankban> dimitern: thanks!
[14:08] <rogpeppe> frankban: reviewed
[14:11] <frankban> rogpeppe: thanks!
[14:12] <frankban> dimitern, rogpeppe: I'll make the requested changes, and then I 'll try to land it. This branch unblocks the GUI and we can re-discuss API implication/changes later. How does it sound?
[14:13] <rogpeppe> frankban: assuming the GUI will be able to change to use a different API if we decide to revert to the original direction, +1
[14:15] <frankban> rogpeppe: cool
[14:15] <dimitern> frankban: what implications?
[14:16] <dimitern> rogpeppe, frankban: I'm ok with depracating that call later than 1.14
[14:17] <dimitern> but at least there should be a // DEPRECATE(vTBD) or something there
[14:17] <rogpeppe> dimitern: +1
[14:19] <frankban> dimitern: yes, Roger suggested it in his review
[14:19] <dimitern> frankban: ok then; my point is - it'll be useful later to search for these DEPRECATE tags
[14:25] <dimitern> rvba: hey
[14:26] <rvba> Hi dimitern.
[14:26] <dimitern> rvba: if you search for all errors in the codebase, you'll see the lowercase-first format; moreover as you said errors can be chained, so they're not a sentence by themselves
[14:27] <rvba> dimitern: right but in this case, it's not an error, it's a unit-test failure message. AFAIK it won't be chained.
[14:27] <rvba> That's why I'm not sure the rule applies here :).
[14:28] <dimitern> rvba: even these are lowercase-first - take a look at c.Logf, c.Fatalf, c.Errorf invocations in tests
[14:28] <dimitern> rvba: i'm aiming for consistency here
[14:28] <rvba> dimitern: ah ok, you're right… it's a bit weird but okay, consistency is good :)
[14:29] <rvba> Thanks for the heads up.
[14:29] <dimitern> rvba: thanks for understanding :)
[14:37] <dimitern> rogpeppe: so you merged https://codereview.appspot.com/12551043/ with 1 LGTM only? (not that i'm complaining much..)
[14:38] <rogpeppe> dimitern: oh
[14:38] <rogpeppe> dimitern: for some reason i was sure i'd had another review of it
[14:38] <dimitern> mgz: can you LGTM that post-factum ? https://codereview.appspot.com/12551043/
[14:39] <rogpeppe> dimitern: could someone else take a look at it, for form's sake?
[14:39] <rogpeppe> mgz, natefinch: ^
[14:39] <mgz> dimitern: sure :)
[14:39] <mgz> oo, it's actually quite a complex one
[14:41] <natefinch> rogpeppe:  I'll give it a quick once over, but I only have about 20 minutes before I have to jet, so it might not be enough time.  Sounds like Martin has it though.
[14:42] <dimitern> mgz: not really, it's mostly restructuring and removals
[14:42] <rogpeppe> natefinch: any remarks about what you see in that time would be appreciated regardless, if you want to have a look
[14:42] <mgz> yeah, looks mostly renaming and shuffling, just a nice big diff :0
[14:48] <natefinch> rogpeppe: GetAnnotationsResults is a struct, not a function?  Oddly named...
[14:49] <rogpeppe> natefinch: the convention in the params package is to declare structs that are parameters to API calls
[14:49] <rogpeppe> natefinch: the argument parameter struct is named after the call (so we might have params.GetAnnotations, for example
[14:49] <rogpeppe> natefinch: the results struct gets a "Results" suffix
[14:50] <rogpeppe> natefinch: i agree that in isolation it looks odd
[14:52] <natefinch> rogpeppe: why use structs for params and returns?
[14:52] <rogpeppe> natefinch: so that we can add extra parameters to API calls without breaking backwards compatibility
[14:52] <natefinch> rogpeppe: Ahh, yeah, good point
[14:52] <rogpeppe> natefinch: it means that all parameters to API calls are named in the json
[14:53] <natefinch> rogpeppe: also a good point :)
=== natefinch is now known as natefinch-afk
[15:47] <rogpeppe> natefinch-afk: you have another revier
[15:47] <rogpeppe> review
=== abentley is now known as abentley-lunch
[16:05] <rogpeppe> dimitern: i've just thought of a potential problem with my Bootstrapper idea
[16:05] <dimitern> rogpeppe: oh?
[16:06] <rogpeppe> dimitern: it assumes that the only reason for calling EnvironProvider.PrepareToBootstrap is if you're about to bootstrap
[16:06] <dimitern> rogpeppe: what else?
[16:06] <rogpeppe> dimitern: but that's not actually true - sync-tools is an example of that
[16:06] <dimitern> rogpeppe: hmm.. it needs the bucket only, but does not bootstrap
[16:07] <rogpeppe> dimitern: because you want to call sync-tools to push tools out to a current env (including verifying and possibly creating the bucket), but then you don't bootstrap
[16:07] <dimitern> rogpeppe: yeah, good point
[16:08] <dimitern> rogpeppe: but this does not invalidate my Initialize proposal, right?
[16:08] <rogpeppe> dimitern: not sure. when would you call Initialize?
[16:09] <dimitern> rogpeppe: when you need to have a bucket or anything else pre-bootstrap
[16:09] <rogpeppe> dimitern: where would the config argument to Initialize come from?
[16:09] <dimitern> rogpeppe: you don't have to call bootstrap afterwards
[16:10] <dimitern> rogpeppe: envs.yaml?
[16:10] <rogpeppe> dimitern: in that case it has the same problem
[16:10] <rogpeppe> dimitern: assuming you always call Initialize before calling Bootstrap
[16:11] <dimitern> rogpeppe: what config do you need to pass to Initialize anyway?
[16:12] <rogpeppe> dimitern: if i call sync-tools, then i call bootstrap, the bootstrap command should see the control-bucket that's been created by calling sync-tools
[16:12] <rogpeppe> dimitern: if Initialize only looks at environments.yaml, and it's called before Bootstrap, then Bootstrap will see a new control bucket
[16:12] <rogpeppe> dimitern: ... not the one that was created in the course of calling sync-tools
[16:14] <rogpeppe> dimitern: i *think* i've got a solution, which applies equally to Initialize or Bootstrapper
[16:14] <dimitern> rogpeppe: i don't think it's sync-tools' job to create a bucket
[16:14] <rogpeppe> dimitern: in which case, how can you use sync-tools?
[16:14] <dimitern> rogpeppe: it has to rely on the provider to do that, just like bootstrap does
[16:15] <rogpeppe> dimitern: ok, assume a totally new juju environment. i've created environments.yaml with my provider credentials but no control bucket
[16:15] <rogpeppe> dimitern: should i be able to call sync-tools now?
[16:15] <dimitern> rogpeppe: yes, and it should check there's no bucket and call initialize
[16:15] <rogpeppe> dimitern: ok, so it's created a new name for the bucket, right?
[16:16] <dimitern> rogpeppe: in fact any such command have to ensure there's a bucket if it needs it
[16:16] <dimitern> rogpeppe: yes, which is then stored in the local attrs
[16:16] <rogpeppe> dimitern: ok, so then I call Bootstrap
[16:16] <rogpeppe> dimitern: Bootstrap calls Initialize
[16:16] <dimitern> rogpeppe: yeah, it sees the bucket from the local attrs and uses that
[16:17] <rogpeppe> dimitern: ah, i thought we were creating new settings each time an environment was bootstrapped
[16:17] <dimitern> rogpeppe: you call bootstrap the command or bootstrap from environs?
[16:17] <rogpeppe> dimitern: the command
[16:17] <dimitern> rogpeppe: let's revisit that a bit
[16:18] <dimitern> rogpeppe: bootstrap creates new local config, if it doesn't exist yet
[16:18] <rogpeppe> dimitern: what if exists but the old bucket name has now been taken?
[16:19] <dimitern> rogpeppe: what of it? is it any different from how is it now?
[16:19] <dimitern> rogpeppe: wait, what? "taken" ? by whom
[16:19] <rogpeppe> dimitern: someone random on the internet
[16:20] <dimitern> rogpeppe: it was created by iniialize when you called sync-tools, and it's not removed, no?
[16:20] <dimitern> rogpeppe: the bootstrap reuses that, given it's already there in the local conf
[16:21] <dimitern> rogpeppe: istm the only way for the bucket to be taken is if it was removed before bootstrap
[16:22] <rogpeppe> dimitern: hmm, i think that might work. we always read local attrs, even when calling Bootstrap. we destroy local attrs when we call destroy-environment
[16:22] <dimitern> rogpeppe: +1
[16:22] <dimitern> rogpeppe: we should also do it when bootstrap fails perhaps
[16:23] <rogpeppe> dimitern: hmm, not sure
[16:23] <dimitern> rogpeppe: i'm not big on that one either
[16:23] <dimitern> rogpeppe: it's just one command away to remove it
[16:23] <rogpeppe> dimitern: yeah
[16:23] <rogpeppe> dimitern: it does means that you can meaningfully call destroy-environment even when you haven't bootstrapped
[16:24] <dimitern> rogpeppe: I think you can do that even now
[16:24] <rogpeppe> dimitern: yeah, probably
[16:24] <dimitern> rogpeppe: yep, just tried - no error
[16:24] <rogpeppe> dimitern: i'm trying to work out the implications of this in a distributed environment
[16:26] <rogpeppe> dimitern: i think this whole thing makes it more crucial that we generate the env's UUID the first time we write out the local attrs
[16:26] <dimitern> rogpeppe: well, having local conf and envs.yaml means you need to provide both of these to a third party wishing to access you env
[16:26] <dimitern> your
[16:27] <rogpeppe> dimitern: i'm thinking of debugging this, when you've got a stale local attrs file because someone else (probably you) has destroyed & re-bootstrapped an environment
[16:27] <dimitern> rogpeppe: worst thing?
[16:27] <dimitern> rogpeppe: juju won't see your env and will ask you to bootstrap it again
[16:28] <dimitern> rogpeppe: but the env itself won't be affected by the stale conf
[16:28] <rogpeppe> dimitern: machine 1: bootstrap; copy local env file to machine 2; destroy; bootstrap;   machine 2: bootstrap
[16:28] <rogpeppe> dimitern: -> two environments
[16:28] <rogpeppe> dimitern: perhaps that's fine though
[16:28] <dimitern> rogpeppe: yep
[16:29] <dimitern> rogpeppe: you know the saying about making things foolproof
[16:29] <rogpeppe> dimitern: or, worse because silent: call sync-tools on machine 2 - it'll upload the tools to a new environment, and say it succeeded
[16:30] <rogpeppe> dimitern: even though you *think* you uploaded to the original env
[16:30] <dimitern> rogpeppe: shouldn't sync-tools fail if you already have the latest?
[16:30] <rogpeppe> dimitern: the sync-tools on machine-2 will re-use the old control bucket
[16:31] <rogpeppe> dimitern: which will have been destroyed earlier
[16:31] <dimitern> rogpeppe: that way, assuming you did sync-tools originally on m1, doing it on m2, after destroying and re-bootstrapping on m1 will succeed
=== tasdomas is now known as tasdomas_afk
[16:31] <dimitern> rogpeppe: yep
[16:31] <rogpeppe> dimitern: no, because the re-bootstrapping on m1 will now be using a new control bucket
[16:32] <dimitern> rogpeppe: we might have to think of juju export or migrate command to share envs like that
[16:33] <rogpeppe> dimitern: well, that's where J**S comes in
[16:33] <dimitern> rogpeppe: or at least store the intent of sharing it somewhere, so we can warn users about destroying shared envs
[16:33] <dimitern> rogpeppe: before that
[16:34] <rogpeppe> dimitern: nah, i think there's nothing useful we can do there
[16:34] <dimitern> rogpeppe: how about a flag "shared" that, if set, will make some commands issue warnings
[16:34] <rogpeppe> dimitern: honestly, i think the user should just be aware
[16:34] <rogpeppe> dimitern: although....
[16:35] <rogpeppe> dimitern: perhaps we could print a message when generating a new environment
[16:35] <dimitern> rogpeppe: juju should be aware as well i think, if doesn't hurt and helps us give more context to an absent-minded users
[16:35] <sidnei> rogpeppe, dimitern: are you guys happy with https://codereview.appspot.com/12235043/ ? im dying to use it
[16:36] <dimitern> sidnei: rogpeppe can say about his review, but I think william had some issues with it
[16:37] <dimitern> sidnei: I'll take a closer look
[16:39] <sidnei> just realized nothing prevents me from merging locally, so. ha.
[16:39] <sidnei> so much nicer.
[16:40] <sidnei> seems like axw addressed william's concerns, unless there were further ones.
[16:42] <dimitern> fwereade: hey
[16:42] <fwereade> dimitern, heyhey
[16:42] <dimitern> fwereade: https://codereview.appspot.com/12235043/ - wanna have a look?
[16:43] <dimitern> fwereade: you already reviewed that earlier
[16:44] <fwereade> dimitern, I started yesterday but distractions happened
[16:44] <fwereade> dimitern, I'll see if I can finish it now
[16:50] <dimitern> rogpeppe: you perhaps as well?
[16:50] <rogpeppe> dimitern: i'm just on it
[16:52] <dimitern> cheers!
=== tasdomas_afk is now known as tasdomas
=== abentley-lunch is now known as abentley
[17:02] <rogpeppe> sidnei: i've just re-reviewed. looks very close to me, but fwereade's MMV
[17:03] <rogpeppe> fwereade: hiya BTW,
[17:04] <rogpeppe> fwereade: hope all's going ok
[17:04] <fwereade> rogpeppe, heyhey
[17:04] <fwereade> rogpeppe, yeah, not too bad
[17:04] <fwereade> rogpeppe, I *might* manage more than one review today but don;t count on it :(
[17:05] <sidnei> thanks!
=== natefinch-afk is now known as natefinch
[17:23] <rogpeppe> natefinch: i made a couple of suggestions in https://codereview.appspot.com/12546043/
[17:23] <natefinch> rogpeppe: yep, was just looking at them
[17:24] <rogpeppe> natefinch: i haven't verified that the Command-wrapping stuff works, but i don't see why it shoudn't and it seems to me a reasonably clear win
[17:25] <dimitern> sidnei: that CL is 2xLGTMed now, so expect it to land soon :)
[17:25] <natefinch> rogpeppe: I liked Andrew's suggestion of generalizing the error handling, but I was going to do it via a defer in each Run command... like this:
[17:25] <natefinch> func (c *AddMachineCommand) Run(ctx *cmd.Context) (err error) {
[17:25] <natefinch> 	defer func() { err = c.HandleError(err, ctx.Stderr) }()
[17:26] <natefinch> rogpeppe: wrapping keeps it all in one spot, which is nice, though
[17:26] <rogpeppe> natefinch: yeah, that's the idea
[17:26] <rogpeppe> natefinch: so if we add new environment commands, they'll just get the benefit automatically
[17:27] <rogpeppe> natefinch: and it doesn't need any new mechanisms in cmd, which is kinda nice
[17:27] <natefinch> rogpeppe: yeah, I didn't want to add more to the Command interface
[17:27] <rogpeppe> natefinch: yeah
[17:27] <rogpeppe> natefinch: it's unnecessary
[17:30] <natefinch> rogpeppe: ok, sounds good. I'll work on that now.
[17:30] <rogpeppe> natefinch: cool, thanks
[17:51]  * rogpeppe has reached eod
[17:51] <rogpeppe> g'night all
=== tasdomas is now known as tasdomas_afk
=== tasdomas_afk is now known as tasdomas
=== tasdomas is now known as tasdomas_afk