bigjoolsdavecheney: does the core team have any plans to set up CI for juju?00:01
davecheneybigjools: yes, we want to set that up00:15
davecheneybut we're still stuck on the dependency management problem00:15
davecheneyat the moment tarmac and lb build recipies are the best we have00:15
bigjoolsdavecheney: right - nobody replied to my email suggesting a way of handling that.00:15
bigjoolsdavecheney: also at the sprint here I want to move gwacl over to the same bot that lands the core00:16
davecheneybigjools: ok, cool00:21
davecheneysorry, stil on da phone00:22
* bigjools picked a hotel for everyone. It's not too late to move it to a grotty backpackers hostel.00:23
davecheneyaxw: did you want to try for a 1.13.1 release tomorrow or friday to pick up debug-hooks ?02:00
davecheneywhen i say 'you', i mean 'me', except when it comes to writing the release notes, in which case i mean 'you' :)02:01
axwdavecheney: 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 any02:01
axwI'll write something up after finishing reviews02:02
davecheneyaxw: it can be as simple as 'debug-hooks' now works02:03
davecheneyif there are any operatoinal diffrences just call them out02:03
davecheneybut it sort of works as expected right ?02:03
axwyeah there shouldn't be any differences02:03
axwI'll read over the docs again to make sure02:03
davecheneyaxw: https://docs.google.com/a/canonical.com/document/d/1XPzmRAM3W7oYYmCGVYB5nR1DZyB16t8qPF68xxmWZho/edit#02:06
axwdavecheney: ta02:07
axwdavecheney: yeah, there are no operational differences between pyjuju and juju-core implementations02:10
axwso I guess your one liner is good enough02:10
davecheneyaxw: marco and I are going to use it release notes or not02:11
axwuse what?02:11
davecheneywe're doing some training next week, north of australia02:11
davecheneyso debug hooks would be usefu;02:12
axwcool :)02:12
davecheneyit is one of the last things keeping people on pyjuju02:12
davecheneythat and the argument about provider specific constraint names02:12
axwdavecheney: has anyone written anything substantial about what's expected of the null/manual provider?03:21
axwe.g. what is it meant to do when a request for a new instance is made?03:22
davecheneyaxw: willimam would have to be the one to answer that03:44
davecheneyit sounds more like a pipe dream03:45
axwdavecheney: ok, ta03:45
axwyou don't think it's feasible?03:45
davecheneybut i guess it operates like a provider where someone has called 'add-machine' to prime th environment03:45
davecheneyaxw i haven't see a spec, just a lot of hyperventilation03:45
davecheneyso i'll reserve judgement03:45
jtvHi wallyworld_ -- could I quiz you a bit more about simplestreams?03:55
wallyworld_jtv: sure, give me a couple of minutes03:55
* jtv gives wallyworld a couple of minutes03:55
wallyworld_jtv: z'up04:11
jtvHi wallyworld_.  I was wondering about the daily vs. release simplestreams.04:14
jtvHow should we arrange configuration for this?04:15
wallyworld_what were you wondering?04:15
jtvHere's what:04:15
wallyworld_i just use the release ones04:15
jtvDefault would get the base URLs for both the release and the daily simplestreams.04:15
wallyworld_i ignore the dailies04:15
jtvThe Stream would be configurable,04:15
jtvand 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 both04:16
wallyworld_as far as i remember04:16
wallyworld_the difference is in the product ids inside the index04:16
jtvIt all depends on how far down that tree we navigate, doesn't it?  Lemme check04:16
wallyworld_my memory might be wrong04:17
jtvI think those need to be separate base URLs.04:17
wallyworld_ah, that looks lke it may have changed since i last looked at it04:18
wallyworld_yes, in that case, the base urls need to be different04:18
jtvBut 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 yet04:18
jtvBecause we want the config to be flexible, but also, simple.04:19
jtvThat explains why searching the providers for "daily" didn't get me anything.  :)04:19
wallyworld_i think including both urls will work04:19
wallyworld_but i think we may also need a user override for base url04:19
jtvIt 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
jtvIIRC 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 understand04:21
jtvTrue... and having an order of preference would be something that belonged in imagemetadata anyway.04:22
jtvSo I'll go with a single config item then: "stream."  It'll default to the releases stream.04:22
wallyworld_sounds good04:22
jtv(I think that stream is called "released" but its index is called "releases" or something like that)04:22
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 part04:23
jtvAh yes, the stream is called "released" but the base URL says "releases".04:23
wallyworld_so we can have image metadata and easily add tools metadata04:23
wallyworld_most of the code is framework, the image andtools bits just define some type specific artifacts04:23
jtvwallyworld_: actually it looks as if maybe imagemetadata.DefaultBaseURL should be an array: [released, daily].04:26
jtvBut absent this same streams configuration in the other providers, that would suddenly make them use daily images.  :/04:27
wallyworld_the daily one could come second? and isn't daily used in the product id? no no matches would occur?04:28
jtvwallyworld_: 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:27
jtvSo 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
jtvIn fact I don't see why any of this should be provider-specific.05:29
wallyworld_jtv: it is provider specific because different providers has their own way of giving out metadata. only CPC's use the default base urls06:02
wallyworld_eg canonistack defines a keystone endpoint06:03
wallyworld_and we want the default urls to be used last so users can override them06:03
jtvI see.06:05
jtvStill, selecting a stream should be pretty standard, no?06:05
=== tasdomas_afk is now known as tasdomas
=== tasdomas is now known as tasdomas_afk
noodles775Thanks 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/1253504307:30
sidneithumper: 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 state07:33
sidneiok, blowing away state/bundles and state/deployer made it get unstuck07:38
=== tasdomas_afk is now known as tasdomas
rogpeppemornin' all07:59
rogpeppeanyone up for giving me another review of this? https://codereview.appspot.com/12473043/08:00
rogpeppeand i'm after any reviews of this: https://codereview.appspot.com/12551043/08:01
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 hours08:04
rogpeppewallyworld_: will do. have fun!08:04
rogpeppemgz: 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
rogpeppedimitern: you've been waiting for this a whle: https://codereview.appspot.com/12551043/08:37
dimiternrogpeppe: cheers!08:38
dimiternrogpeppe: ian sent me an email that he's going to miss today and tomorrow's standup due to soccer08:41
rogpeppedimitern: ok08:41
dimiternrogpeppe: did you manage to chat with him yesterday?08:41
rogpeppedimitern: no08:41
dimiternrogpeppe: well, it's friday then08:42
rogpeppedimitern: i08:42
rogpeppedimitern: i might be able to catch him today after soccer08:42
dimiternrogpeppe: ah, ok08:43
axwrvba: ping?08:53
rvbaHi axw08:53
axwrvba: hi. regarding the NetworkConfigurationSet thing...08:54
axwNewNetworkConfigurationSet takes a slice, which you're passing in as nil08:54
axwit then takes the address08:54
axwso it's never going to be a nil pointer08:54
axwrvba: nto sure if this is really a problem or not, I'm just going by what the gwacl comment says08:56
rvbaaxw: 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:56
rvbaaxw: 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
axwI will LGTM now08:58
rvbaSorry *I* wasn't very clear before :)08:58
dimiternrogpeppe: I particularly like the "oneLife" method09:01
dimiternrogpeppe: I don't think you merged my trunk changes into your branch though09:05
rogpeppedimitern: no, good point - i'll do that09:05
dimiternrogpeppe: reviewed09:13
rogpeppedimitern: ta!09:13
rogpeppedimitern: 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 them09:16
dimiternrogpeppe: thanks!09:17
dimiternrogpeppe: I suspected it was a bit awkward to implement them, but they still test a valid case, which shows we handle state errors correctly09:18
noodles775dimitern: 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/17876609:21
_mup_Bug #1209112: deployer stuck in unrecoverable loop after power loss <juju-core:New> <https://launchpad.net/bugs/1209112>09:21
dimiternsidnei: sorry, I don't really understand what was the issue there09:27
sidneidimitern: the zip file in the state dir was corrupted09:27
sidneidimitern: or at least that's my understanding09:27
dimiternsidnei: how did that happen?09:27
sidneidimitern: local deployer, ran out of battery in my laptop09:27
dimiternsidnei: but it seems the issue was with the uniter not downloading the complete zip file, not the deployer09:29
sidneidimitern: i probably got them confused, yes09:30
dimiternsidnei: I edited the description, but otherwise it seems like a genuine uniter issue09:32
dimiternsidnei: thanks for the report09:32
dimiternrogpeppe: unit agents are now deployed with APIInfo, right?09:44
rogpeppedimitern: yes09:45
rogpeppedimitern: they even connect to the API now09:45
dimiternrogpeppe: great, so we have half of the blueprint implemented now :)09:45
noodles775axw: 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:52
axwnoodles775: not sure what happened there, it looks unrelated tho.  Perhaps a race condition in another test. I'll reapprove09:54
dimiternrogpeppe: I though you fixed that http://paste.ubuntu.com/5958349/10:06
=== axw is now known as axw_
axw_noodles775: it worked this time10:09
rogpeppedimitern: i'm still waiting for a second review on the branch10:09
dimiternrogpeppe: no, I meant TestMachineWatch10:09
dimiternaxw_: it's intermittent10:09
rogpeppedimitern: that's what i'm talking about10:09
rogpeppedimitern: https://codereview.appspot.com/12352044/10:09
dimiternrogpeppe: ah, I see10:09
dimiternaxw_: can you review that please? ^^10:10
axw_yup, may be beyond me tho10:10
rogpeppeaxw_: a good opportunity to delve into the details of the state/watcher code :-)10:10
dimiternaxw_: take a look at State.Sync and State.StartSync - that's the crux of the problem10:11
axw_dimitern: thanks10:11
noodles775axw_: Thanks - merged that time.10:11
axw_noodles775: nps10:12
rogpeppeaxw_: 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:12
dimiternnoodles775: 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 checkouts10:13
noodles775dimitern: 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
dimiternnoodles775: yeah, that part in CONTRIBUTING needs to be updated10:18
noodles775dimitern: 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:19
noodles775For 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:20
dimiternnoodles775: 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 last10:21
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:25
noodles775dimitern: what about go - is godeb install 1.1.1 preferred for a dev env, or a PPA?10:28
dimiternnoodles775: we all should be using the golang version from ppa:james-page/golang-backports10:29
dimiternnoodles775: it's 1.1.1 + some post-release fixes, that will be like that until we backport it to the archive10:29
jamespagedimitern, 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-backports10:31
jamespagedo you want to sync that to your official ~juju golang PPA?10:31
jamespagenoodles775, dimitern: this PPA would be better - https://launchpad.net/~juju/+archive/golang10:31
dimiternjamespage: ah, ok, good to know10:32
* dimitern bbiab10:32
jamespagethats what we are using todo all of the juju-core backport builds10:32
noodles775Great - thanks jamespage10:32
jamespagedavecheney, still awake?10:32
mgzjamespage: yup, we should10:36
mgzcopied acriss10:46
mgzremoved the saucy one as that doesn't need backporting10:47
=== marcoceppi_ is now known as marcoceppi
dimiternrogpeppe, mgz, natefinch: standup11:31
rogpeppemgz: you still there?11:35
mgzhow long was I talking for when g+ decided to log me out11:36
rogpeppemgz: not v long11:36
natefinchrogpeppe, 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
rogpeppenatefinch: i think we only do that in tests12:18
natefinchrogpeppe: ok, that's fair12:19
rogpeppedimitern: 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
dimiternrogpeppe: great, thanks! will take a look in a bit12:21
rogpeppei wondered why the network was down. turned out that Carmen had accidentally unplugged the ethernet switch when cleaning12:43
dimiternnatefinch, rogpeppe: reviewed12:56
natefinchdimitern:  thanks!12:56
rogpeppedimitern: thanks!12:56
rogpeppedimitern: the TODO now lives in the FindEntity method12:57
dimiternrogpeppe: ah, ok I missed that then12:57
* rogpeppe goes for lunch13:04
frankbancould anyone please take a look at https://codereview.appspot.com/12522043? thanks!13:40
dimiternfrankban: i will13:45
frankbandimitern: thank you13:48
dimiternfrankban: reviewed13:58
frankbandimitern: thanks!14:00
rogpeppefrankban: reviewed14:08
frankbanrogpeppe: thanks!14:11
frankbandimitern, 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:12
rogpeppefrankban: assuming the GUI will be able to change to use a different API if we decide to revert to the original direction, +114:13
frankbanrogpeppe: cool14:15
dimiternfrankban: what implications?14:15
dimiternrogpeppe, frankban: I'm ok with depracating that call later than 1.1414:16
dimiternbut at least there should be a // DEPRECATE(vTBD) or something there14:17
rogpeppedimitern: +114:17
frankbandimitern: yes, Roger suggested it in his review14:19
dimiternfrankban: ok then; my point is - it'll be useful later to search for these DEPRECATE tags14:19
dimiternrvba: hey14:25
rvbaHi dimitern.14:26
dimiternrvba: 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 themselves14:26
rvbadimitern: 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
rvbaThat's why I'm not sure the rule applies here :).14:27
dimiternrvba: even these are lowercase-first - take a look at c.Logf, c.Fatalf, c.Errorf invocations in tests14:28
dimiternrvba: i'm aiming for consistency here14:28
rvbadimitern: ah ok, you're right… it's a bit weird but okay, consistency is good :)14:28
rvbaThanks for the heads up.14:29
dimiternrvba: thanks for understanding :)14:29
dimiternrogpeppe: so you merged https://codereview.appspot.com/12551043/ with 1 LGTM only? (not that i'm complaining much..)14:37
rogpeppedimitern: oh14:38
rogpeppedimitern: for some reason i was sure i'd had another review of it14:38
dimiternmgz: can you LGTM that post-factum ? https://codereview.appspot.com/12551043/14:38
rogpeppedimitern: could someone else take a look at it, for form's sake?14:39
rogpeppemgz, natefinch: ^14:39
mgzdimitern: sure :)14:39
mgzoo, it's actually quite a complex one14:39
natefinchrogpeppe:  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:41
dimiternmgz: not really, it's mostly restructuring and removals14:42
rogpeppenatefinch: any remarks about what you see in that time would be appreciated regardless, if you want to have a look14:42
mgzyeah, looks mostly renaming and shuffling, just a nice big diff :014:42
natefinchrogpeppe: GetAnnotationsResults is a struct, not a function?  Oddly named...14:48
rogpeppenatefinch: the convention in the params package is to declare structs that are parameters to API calls14:49
rogpeppenatefinch: the argument parameter struct is named after the call (so we might have params.GetAnnotations, for example14:49
rogpeppenatefinch: the results struct gets a "Results" suffix14:49
rogpeppenatefinch: i agree that in isolation it looks odd14:50
natefinchrogpeppe: why use structs for params and returns?14:52
rogpeppenatefinch: so that we can add extra parameters to API calls without breaking backwards compatibility14:52
natefinchrogpeppe: Ahh, yeah, good point14:52
rogpeppenatefinch: it means that all parameters to API calls are named in the json14:52
natefinchrogpeppe: also a good point :)14:53
=== natefinch is now known as natefinch-afk
rogpeppenatefinch-afk: you have another revier15:47
=== abentley is now known as abentley-lunch
rogpeppedimitern: i've just thought of a potential problem with my Bootstrapper idea16:05
dimiternrogpeppe: oh?16:05
rogpeppedimitern: it assumes that the only reason for calling EnvironProvider.PrepareToBootstrap is if you're about to bootstrap16:06
dimiternrogpeppe: what else?16:06
rogpeppedimitern: but that's not actually true - sync-tools is an example of that16:06
dimiternrogpeppe: hmm.. it needs the bucket only, but does not bootstrap16:06
rogpeppedimitern: 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 bootstrap16:07
dimiternrogpeppe: yeah, good point16:07
dimiternrogpeppe: but this does not invalidate my Initialize proposal, right?16:08
rogpeppedimitern: not sure. when would you call Initialize?16:08
dimiternrogpeppe: when you need to have a bucket or anything else pre-bootstrap16:09
rogpeppedimitern: where would the config argument to Initialize come from?16:09
dimiternrogpeppe: you don't have to call bootstrap afterwards16:09
dimiternrogpeppe: envs.yaml?16:10
rogpeppedimitern: in that case it has the same problem16:10
rogpeppedimitern: assuming you always call Initialize before calling Bootstrap16:10
dimiternrogpeppe: what config do you need to pass to Initialize anyway?16:11
rogpeppedimitern: if i call sync-tools, then i call bootstrap, the bootstrap command should see the control-bucket that's been created by calling sync-tools16:12
rogpeppedimitern: if Initialize only looks at environments.yaml, and it's called before Bootstrap, then Bootstrap will see a new control bucket16:12
rogpeppedimitern: ... not the one that was created in the course of calling sync-tools16:12
rogpeppedimitern: i *think* i've got a solution, which applies equally to Initialize or Bootstrapper16:14
dimiternrogpeppe: i don't think it's sync-tools' job to create a bucket16:14
rogpeppedimitern: in which case, how can you use sync-tools?16:14
dimiternrogpeppe: it has to rely on the provider to do that, just like bootstrap does16:14
rogpeppedimitern: ok, assume a totally new juju environment. i've created environments.yaml with my provider credentials but no control bucket16:15
rogpeppedimitern: should i be able to call sync-tools now?16:15
dimiternrogpeppe: yes, and it should check there's no bucket and call initialize16:15
rogpeppedimitern: ok, so it's created a new name for the bucket, right?16:15
dimiternrogpeppe: in fact any such command have to ensure there's a bucket if it needs it16:16
dimiternrogpeppe: yes, which is then stored in the local attrs16:16
rogpeppedimitern: ok, so then I call Bootstrap16:16
rogpeppedimitern: Bootstrap calls Initialize16:16
dimiternrogpeppe: yeah, it sees the bucket from the local attrs and uses that16:16
rogpeppedimitern: ah, i thought we were creating new settings each time an environment was bootstrapped16:17
dimiternrogpeppe: you call bootstrap the command or bootstrap from environs?16:17
rogpeppedimitern: the command16:17
dimiternrogpeppe: let's revisit that a bit16:17
dimiternrogpeppe: bootstrap creates new local config, if it doesn't exist yet16:18
rogpeppedimitern: what if exists but the old bucket name has now been taken?16:18
dimiternrogpeppe: what of it? is it any different from how is it now?16:19
dimiternrogpeppe: wait, what? "taken" ? by whom16:19
rogpeppedimitern: someone random on the internet16:19
dimiternrogpeppe: it was created by iniialize when you called sync-tools, and it's not removed, no?16:20
dimiternrogpeppe: the bootstrap reuses that, given it's already there in the local conf16:20
dimiternrogpeppe: istm the only way for the bucket to be taken is if it was removed before bootstrap16:21
rogpeppedimitern: hmm, i think that might work. we always read local attrs, even when calling Bootstrap. we destroy local attrs when we call destroy-environment16:22
dimiternrogpeppe: +116:22
dimiternrogpeppe: we should also do it when bootstrap fails perhaps16:22
rogpeppedimitern: hmm, not sure16:23
dimiternrogpeppe: i'm not big on that one either16:23
dimiternrogpeppe: it's just one command away to remove it16:23
rogpeppedimitern: yeah16:23
rogpeppedimitern: it does means that you can meaningfully call destroy-environment even when you haven't bootstrapped16:23
dimiternrogpeppe: I think you can do that even now16:24
rogpeppedimitern: yeah, probably16:24
dimiternrogpeppe: yep, just tried - no error16:24
rogpeppedimitern: i'm trying to work out the implications of this in a distributed environment16:24
rogpeppedimitern: i think this whole thing makes it more crucial that we generate the env's UUID the first time we write out the local attrs16:26
dimiternrogpeppe: well, having local conf and envs.yaml means you need to provide both of these to a third party wishing to access you env16:26
rogpeppedimitern: 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 environment16:27
dimiternrogpeppe: worst thing?16:27
dimiternrogpeppe: juju won't see your env and will ask you to bootstrap it again16:27
dimiternrogpeppe: but the env itself won't be affected by the stale conf16:28
rogpeppedimitern: machine 1: bootstrap; copy local env file to machine 2; destroy; bootstrap;   machine 2: bootstrap16:28
rogpeppedimitern: -> two environments16:28
rogpeppedimitern: perhaps that's fine though16:28
dimiternrogpeppe: yep16:28
dimiternrogpeppe: you know the saying about making things foolproof16:29
rogpeppedimitern: or, worse because silent: call sync-tools on machine 2 - it'll upload the tools to a new environment, and say it succeeded16:29
rogpeppedimitern: even though you *think* you uploaded to the original env16:30
dimiternrogpeppe: shouldn't sync-tools fail if you already have the latest?16:30
rogpeppedimitern: the sync-tools on machine-2 will re-use the old control bucket16:30
rogpeppedimitern: which will have been destroyed earlier16:31
dimiternrogpeppe: that way, assuming you did sync-tools originally on m1, doing it on m2, after destroying and re-bootstrapping on m1 will succeed16:31
=== tasdomas is now known as tasdomas_afk
dimiternrogpeppe: yep16:31
rogpeppedimitern: no, because the re-bootstrapping on m1 will now be using a new control bucket16:31
dimiternrogpeppe: we might have to think of juju export or migrate command to share envs like that16:32
rogpeppedimitern: well, that's where J**S comes in16:33
dimiternrogpeppe: or at least store the intent of sharing it somewhere, so we can warn users about destroying shared envs16:33
dimiternrogpeppe: before that16:33
rogpeppedimitern: nah, i think there's nothing useful we can do there16:34
dimiternrogpeppe: how about a flag "shared" that, if set, will make some commands issue warnings16:34
rogpeppedimitern: honestly, i think the user should just be aware16:34
rogpeppedimitern: although....16:34
rogpeppedimitern: perhaps we could print a message when generating a new environment16:35
dimiternrogpeppe: juju should be aware as well i think, if doesn't hurt and helps us give more context to an absent-minded users16:35
sidneirogpeppe, dimitern: are you guys happy with https://codereview.appspot.com/12235043/ ? im dying to use it16:35
dimiternsidnei: rogpeppe can say about his review, but I think william had some issues with it16:36
dimiternsidnei: I'll take a closer look16:37
sidneijust realized nothing prevents me from merging locally, so. ha.16:39
sidneiso much nicer.16:39
sidneiseems like axw addressed william's concerns, unless there were further ones.16:40
dimiternfwereade: hey16:42
fwereadedimitern, heyhey16:42
dimiternfwereade: https://codereview.appspot.com/12235043/ - wanna have a look?16:42
dimiternfwereade: you already reviewed that earlier16:43
fwereadedimitern, I started yesterday but distractions happened16:44
fwereadedimitern, I'll see if I can finish it now16:44
dimiternrogpeppe: you perhaps as well?16:50
rogpeppedimitern: i'm just on it16:50
=== tasdomas_afk is now known as tasdomas
=== abentley-lunch is now known as abentley
rogpeppesidnei: i've just re-reviewed. looks very close to me, but fwereade's MMV17:02
rogpeppefwereade: hiya BTW,17:03
rogpeppefwereade: hope all's going ok17:04
fwereaderogpeppe, heyhey17:04
fwereaderogpeppe, yeah, not too bad17:04
fwereaderogpeppe, I *might* manage more than one review today but don;t count on it :(17:04
=== natefinch-afk is now known as natefinch
rogpeppenatefinch: i made a couple of suggestions in https://codereview.appspot.com/12546043/17:23
natefinchrogpeppe: yep, was just looking at them17:23
rogpeppenatefinch: 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 win17:24
dimiternsidnei: that CL is 2xLGTMed now, so expect it to land soon :)17:25
natefinchrogpeppe: 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
natefinchfunc (c *AddMachineCommand) Run(ctx *cmd.Context) (err error) {17:25
natefinchdefer func() { err = c.HandleError(err, ctx.Stderr) }()17:25
natefinchrogpeppe: wrapping keeps it all in one spot, which is nice, though17:26
rogpeppenatefinch: yeah, that's the idea17:26
rogpeppenatefinch: so if we add new environment commands, they'll just get the benefit automatically17:26
rogpeppenatefinch: and it doesn't need any new mechanisms in cmd, which is kinda nice17:27
natefinchrogpeppe: yeah, I didn't want to add more to the Command interface17:27
rogpeppenatefinch: yeah17:27
rogpeppenatefinch: it's unnecessary17:27
natefinchrogpeppe: ok, sounds good. I'll work on that now.17:30
rogpeppenatefinch: cool, thanks17:30
* rogpeppe has reached eod17:51
rogpeppeg'night all17:51
=== tasdomas is now known as tasdomas_afk
=== tasdomas_afk is now known as tasdomas
=== tasdomas is now known as tasdomas_afk

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