wallyworld | arosales: found what's happening - it works if you use the --upload-tools open. not using that option causes a code path to be run which resets the current client credentials. this didn't matter previously but does now | 00:13 |
---|---|---|
wallyworld | s/open/option | 00:13 |
wallyworld | arosales: ping me when you are back online and we can discuss a resolution | 00:15 |
arosales | wallyworld, and --upload-tools requires building from source? | 00:15 |
wallyworld | yes | 00:15 |
arosales | wallyworld, no go for OSCON | 00:15 |
wallyworld | yeah | 00:15 |
wallyworld | i can work on a fix | 00:15 |
wallyworld | but i guess that means cutting a new release | 00:16 |
arosales | wallyworld, we can't have charm school attendees building from source :-/ Kind of takes away from the juju simplicity | 00:16 |
wallyworld | agreed | 00:16 |
arosales | and a bug fix is in the same realm too :-/ | 00:16 |
* arosales not sure how difficult it would be to do a 1.11.3.1 release with just your bug fix . . . | 00:17 | |
wallyworld | i guess a bug fix is more palatable though | 00:17 |
wallyworld | that should be possible | 00:18 |
arosales | wallyworld, but I guess you need to find the issue on the cred reset too | 00:19 |
wallyworld | arosales: i found what's causing it - not uploading tools triggers a code path that resets the creds. it didn't matter before but does now. i just need to figure out a fix | 00:20 |
arosales | wallyworld, ok. I'll leave you too that and we can sync with dave when he comes on line with possible solutions | 00:21 |
wallyworld | arosales: sounds good. i have a simple fix i can do, just need to think if it's the best way | 00:21 |
arosales | fwiw I filed bug https://bugs.launchpad.net/juju-core/+bug/1203908 | 00:23 |
_mup_ | Bug #1203908: 1.11.3 does not bootstrap on HP Cloud <juju-core:New> <https://launchpad.net/bugs/1203908> | 00:23 |
arosales | wallyworld, I am going to step away from the keyboard for a few, but I'll check back online later | 00:32 |
wallyworld | ok | 00:32 |
axw | hello... who's awake? | 01:16 |
axw | davecheney: morning | 01:17 |
davecheney | axw: hey | 01:17 |
thumper | hi axw | 01:17 |
axw | thumper: hiya | 01:17 |
davecheney | so, dependencies really grind my gears | 01:17 |
thumper | axw: do you know duflu from elsewhere? | 01:17 |
davecheney | i'm tired to seeing jtv have to ask for the bot to be updated | 01:18 |
axw | thumper: heh :) yes, we worked together at Micromuse and IBM | 01:18 |
davecheney | i'm going to prpose a script that does the checkout for it | 01:18 |
davecheney | so the bot checks out the code | 01:18 |
davecheney | runs the script which gets the deps | 01:18 |
davecheney | job done | 01:18 |
thumper | heh... if I hadn't just gone to the gym I'd be worried... I got the shakes in my hands | 01:18 |
thumper | amusing to watch | 01:18 |
thumper | first workout in two weeks due to illness | 01:19 |
thumper | felt good | 01:19 |
davecheney | thumper: man flu, or real flu | 01:19 |
davecheney | ? | 01:19 |
thumper | wasn't flu | 01:19 |
thumper | just a persistent cough and fever | 01:20 |
thumper | kid flu passed on | 01:20 |
thumper | can't work out coughing your guts out | 01:20 |
thumper | however managed the entire workout without one single cough | 01:20 |
thumper | so I feel over it now | 01:20 |
davecheney | sgtm | 01:21 |
axw | So, I want to fix a bug I found yesterday in juju-core. Can someone give me a brief walkthrough of creating bugfix/feature branches on LP? | 01:21 |
thumper | axw: happy to | 01:21 |
thumper | but I need to grab a smoothie first | 01:21 |
thumper | axw: gimmie 5 minutes? | 01:21 |
axw | nps | 01:21 |
axw | thanks | 01:21 |
thumper | axw: then we can do a google hangout? | 01:21 |
thumper | axw: are you set up for hangouts? | 01:21 |
axw | thumper: sounds good | 01:21 |
davecheney | can I get in on this one ? | 01:22 |
axw | thumper: should I have G+ on my Canonical account? | 01:22 |
davecheney | or at least listen in | 01:22 |
davecheney | i dunno if this laptop has a mike | 01:22 |
thumper | axw: yep | 01:22 |
thumper | davecheney: yes happy to have you listen in | 01:22 |
axw | I'll get it set up while you get your drink | 01:22 |
davecheney | cool | 01:22 |
davecheney | it may not work | 01:22 |
thumper | axw: also make sure you have your ssh key up on LP to start | 01:22 |
davecheney | this coworking space has the smallest internet known to man | 01:22 |
thumper | or wait if you need a walk through on that | 01:22 |
axw | yeah that's all done | 01:22 |
thumper | ack | 01:22 |
thumper | back in 5 | 01:22 |
wallyworld | davecheney: ping | 01:23 |
davecheney | wallyworld: hey | 01:23 |
davecheney | saw your mail | 01:23 |
davecheney | thanks for replying | 01:23 |
wallyworld | the release - we need to cut a 1.11.3.1 | 01:23 |
wallyworld | but we need to create a branch and series | 01:23 |
wallyworld | first | 01:23 |
wallyworld | so i can backport | 01:23 |
davecheney | _if_ we do a 1.11.3 we _really shold do it from the 1.11.3 tag | 01:23 |
wallyworld | yes | 01:23 |
davecheney | wallyworld: or we could just tag what we have now as 1.11.4 | 01:23 |
wallyworld | hence why we need to do a series and branch | 01:23 |
davecheney | i think that is simpler | 01:23 |
davecheney | wallyworld: unless you want 1.11.3 to be 1.12 | 01:24 |
davecheney | axw: sorry about these arse backwards version numbers | 01:24 |
wallyworld | davecheney: well, 1.11.3 is broken right now | 01:24 |
davecheney | for openstack yes | 01:24 |
davecheney | but 1.11.2 was broken in other ways | 01:24 |
davecheney | i don't think that is cause to deviate from what we've done in thepast | 01:25 |
davecheney | i'd be happier to tag trun 1.11.4 | 01:25 |
davecheney | start 1.11.5 | 01:25 |
davecheney | and then 1.11.4 can become the new stable branch | 01:25 |
wallyworld | davecheney: there are guys at OSCON who need a package version to work | 01:25 |
davecheney | that is why I want to do is as I said aboce | 01:25 |
davecheney | i know that will be the fastest way to get it going | 01:25 |
wallyworld | ok. i'll propose a branch | 01:26 |
wallyworld | mp | 01:26 |
davecheney | kk | 01:26 |
wallyworld | with the fix | 01:26 |
davecheney | wallyworld: are you your keys also know to the bot ? | 01:26 |
davecheney | i might have to ask you to do the tag for me | 01:26 |
wallyworld | hope so | 01:26 |
wallyworld | i'd also want to longer term sort out if we need to change SetConfig() not to be dumb | 01:26 |
wallyworld | i've never tried | 01:27 |
davecheney | wallyworld: i can send you the words | 01:27 |
davecheney | you just have to type them | 01:27 |
wallyworld | davecheney: sure. i mean that i'm not sure if my keys work | 01:27 |
wallyworld | but i'm more than happy to try | 01:27 |
* davecheney throws a chair | 01:28 | |
davecheney | fuck this bot | 01:28 |
davecheney | what fucking good is it if we have to wait for jam do do our commits for us ?!?! | 01:29 |
wallyworld | davecheney: i'll also commit another bug fix that is waiting to land (will help the guys using quantum) | 01:29 |
davecheney | sgtm | 01:29 |
axw | wallyworld: are you fixing the openstack bootstrap bug? | 01:29 |
wallyworld | axw: yes indeed | 01:29 |
axw | ok | 01:29 |
axw | sorry I'd have proposed a fix yesterday, but I wasn't sure how yet :) | 01:30 |
wallyworld | axw: hey, no problem :-) i didn't know it was broken till i found out on irc today | 01:30 |
thumper | davecheney, axw: https://plus.google.com/hangouts/_/ee0597b40c9b901559b872299a2b1678184b8e6a?hl=en | 01:32 |
davecheney | here goes nothing | 01:35 |
wallyworld | arosales: you back? | 01:37 |
wallyworld | davecheney: thumper: when you are free https://codereview.appspot.com/11703043 | 01:47 |
davecheney | wallyworld: will you punch me if I ask for a test ? | 01:50 |
wallyworld | davecheney: yes. the live tests would have failed | 01:50 |
davecheney | +1 then | 01:50 |
davecheney | LGTM | 01:50 |
davecheney | approved | 01:50 |
davecheney | commit that shit | 01:50 |
wallyworld | thanks :-) | 01:50 |
wallyworld | thumper: you able to +1 that mp so i can get it landed for the 11.4 release? | 02:24 |
thumper | davecheney: wanna +1 my mega state.Tools removal branch | 02:24 |
thumper | wallyworld: sure, link me up | 02:24 |
axw | davecheney: should I have an account for leankit? | 02:25 |
wallyworld | https://codereview.appspot.com/11703043 | 02:25 |
davecheney | axw: yes, i don't know how to create one | 02:26 |
davecheney | axw: write to mramm and ask for one | 02:26 |
axw | ok | 02:26 |
davecheney | sorry it's been a while | 02:27 |
davecheney | i can't remember how I came by mine | 02:27 |
hazmat | edge phones going fast | 02:34 |
davecheney | 12 hours til the price goes up | 02:35 |
wallyworld | thumper: do you want me to beg? | 02:39 |
bigjools | wallyworld: you should be used to begging after years of marriage | 02:41 |
wallyworld | yes indeed | 02:41 |
wallyworld | bigjools: like you can talk anyway | 02:41 |
bigjools | we're all whipped my friend | 02:41 |
wallyworld | davecheney: waiting on a 2nd +1 - maybe i should just land it so the release can progress | 02:47 |
thumper | wallyworld: sorry, no begging needed, but house guest arrived | 02:51 |
wallyworld | np | 02:51 |
wallyworld | but i can if you want | 02:52 |
wallyworld | i'm quite good at it | 02:52 |
thumper | done | 02:52 |
thumper | wallyworld: ok, beg | 02:52 |
* thumper waits | 02:52 | |
* wallyworld is down on his knees | 02:53 | |
thumper | done | 02:53 |
davecheney | wallyworld: just land that shit | 02:53 |
wallyworld | than you | 02:53 |
davecheney | if anyone complains, the can blame me | 02:54 |
thumper | davecheney: I +1ed it | 02:56 |
thumper | davecheney: next time go "LGTM - trivial" | 02:56 |
thumper | although jam has suggested going with just one review to land | 02:56 |
thumper | which would unblock people | 02:57 |
thumper | davecheney: like this trivial one: https://codereview.appspot.com/11561044/ | 02:57 |
davecheney | thumper: trivial my arse | 02:58 |
davecheney | it's larger than the fucking codebase | 02:58 |
arosales | wallyworld, back | 03:05 |
davecheney | wallyworld: $ bzr tag -d bzr+ssh://go-bot@bazaar.launchpad.net/~goose-bot/goose/trunk | 03:06 |
davecheney | -r1494 juju-1.11.3 | 03:06 |
davecheney | ^ magic to tag 1.11.4 | 03:06 |
davecheney | obvoiusly with a different tag name and revno | 03:06 |
wallyworld | arosales: a new release is being built. i couldn't fully test bootstrapping because i don't have permission to access the compute nodes. but the original promlem is solved | 03:06 |
wallyworld | davecheney: will do. bot is building now | 03:07 |
davecheney | wallyworld: thanks mate | 03:08 |
arosales | wallyworld, thats good news. Thanks for working on it. I saw you had to beg too. I appreciate that type of commitment ;-) | 03:12 |
davecheney | https://bugs.launchpad.net/bugs/1203935 | 03:13 |
_mup_ | Bug #1203935: Some EC2 instance prices specified in octal <juju-core:New> <https://launchpad.net/bugs/1203935> | 03:13 |
arosales | wallyworld, sorry I didn't fully parse, "I don't have permission to access compute nodes." | 03:13 |
davecheney | facepalm | 03:13 |
wallyworld | arosales: np. hopefully it's no too much inconvenince for the OSCON guys | 03:13 |
wallyworld | arosales: when i bootstrapped, it said there image id didn't exist. from the web console, i click on the compute links and it says i have no permission | 03:14 |
arosales | wallyworld, hmm your a compute admin, let me make you an image admin too | 03:15 |
arosales | wallyworld, made you an image mange admin too. | 03:16 |
davecheney | axw: do yo uwant to https://bugs.launchpad.net/bugs/1203935 | 03:16 |
_mup_ | Bug #1203935: Some EC2 instance prices specified in octal <juju-core:New> <https://launchpad.net/bugs/1203935> | 03:16 |
davecheney | take a look at this bug | 03:16 |
wallyworld | arosales: thanks, i'll try again in a bit. but i'm sure it will be ok now | 03:16 |
davecheney | might be a easly one to work your the kinks on | 03:16 |
axw | davecheney: sure | 03:16 |
arosales | bigjools, will azure be in 11.4? | 03:18 |
davecheney | oh son of a bitch | 03:18 |
arosales | 1.11.4 that is | 03:18 |
davecheney | i'll have to commit it out | 03:18 |
davecheney | comment it out again | 03:18 |
arosales | davecheney, bigjools was saying it was working | 03:18 |
bigjools | why don't you make release branches? this is stupid | 03:18 |
bigjools | arosales: no I didn't! | 03:18 |
arosales | bigjools, :-) | 03:18 |
davecheney | bigjools: i don't have a good reason for why we don't have this | 03:19 |
arosales | bigjools, ah reading your email now I see you had to manually deploy | 03:19 |
bigjools | release branches or config flags would help here | 03:19 |
arosales | bigjools, I thought if I said it enough it would come true | 03:19 |
bigjools | arosales: it will, soon :) | 03:20 |
arosales | bigjools, its good progress | 03:20 |
arosales | davecheney, bigjools, hello btw | 03:21 |
arosales | so this is where all the fun happens :-) | 03:21 |
bigjools | o/ | 03:21 |
arosales | I should join this channel more often when I am up late | 03:21 |
davecheney | arosales: c'mon man, i joined your eco channel, even when #juju-dev is where it is at | 03:22 |
wallyworld | arosales: works :-) | 03:22 |
wallyworld | just bootstrapped on hp cloud | 03:22 |
arosales | davecheney, well said | 03:22 |
arosales | wallyworld, good to hear | 03:23 |
bigjools | davecheney: anyway not sure you need to remove azure again | 03:23 |
bigjools | it works w/o go-curl now | 03:23 |
davecheney | bigjools: ok | 03:23 |
davecheney | excellent | 03:23 |
davecheney | the biuld recipe always pulls the latest deps | 03:24 |
bigjools | excellent apart from the huge fork of 1.0.1 Go code in gwacl | 03:24 |
wallyworld | davecheney: that tag command above - it says goose but surely you meant juju-core? or i guess we need to tag all the dependencies too? | 03:25 |
davecheney | wallyworld: i just copied that from what gz sent me | 03:25 |
davecheney | if that isn't what he ran | 03:25 |
davecheney | then he's a liar | 03:25 |
davecheney | wallyworld: are you able to put my key on the bot so I can do this myself ? | 03:26 |
wallyworld | i don't think so. i'll chck | 03:26 |
davecheney | shazbut! | 03:26 |
wallyworld | dosn't appear so | 03:28 |
wallyworld | i'll tag both goose and juju-core | 03:29 |
wallyworld | davecheney: tags done for goose rev 99 and juju-core rev 1514. and yes, that command was slightly incorrect | 03:31 |
davecheney | wallyworld: thanks mate | 03:32 |
wallyworld | np. hopefully it will build all ok | 03:32 |
davecheney | what could possibly go wrong | 03:37 |
davecheney | wallyworld: while i'm screwing with this | 03:37 |
davecheney | can you please make sure those two bugs are assigned to 1.11.4 and stuff | 03:37 |
wallyworld | ah yes, will do | 03:37 |
davecheney | ta muchly | 03:37 |
davecheney | bollocks, have to do release notes as well | 03:38 |
arosales | davecheney, just +2 bugs fixes from 1.11.3 correct? | 03:39 |
davecheney | arosales: yup | 03:40 |
arosales | fwiw I appreciate the emergency 1.11.4 build | 03:44 |
davecheney | no worries | 03:45 |
wallyworld | davecheney: arosales: there was a bug fixed for 1.11.3 not tagged as such, so i added it to 1.11.4 so folks had visibility to it. bug 1195223 | 03:45 |
_mup_ | Bug #1195223: juju all-machines.log is repetitive and grows unbounded <juju-core:Fix Committed by wallyworld> <https://launchpad.net/bugs/1195223> | 03:45 |
davecheney | wallyworld: did I miss that one ? | 03:46 |
davecheney | was it linked from the leankit card ? | 03:46 |
davecheney | this is the way i do the release notes | 03:46 |
arosales | btw, who let 1.11.3 go out the door with out it bootstrapping on hp cloud ;-) | 03:46 |
wallyworld | davecheney: no, it was my fault. i didn't assign it to any milestone | 03:46 |
wallyworld | arosales: those of us without hp cloud creds :-) | 03:47 |
davecheney | 1. read cards in leankit, if they are bugs and have an lp link I tried to reframe the bug in the release notes | 03:47 |
davecheney | if they sort of look like internall stuff, like the containers work, i omit them from the release notes | 03:47 |
wallyworld | davecheney: i suspect there was a card on leankit but the bug number may not have been there. not sure now | 03:47 |
davecheney | no worries | 03:47 |
davecheney | noone died | 03:47 |
arosales | wallyworld, ah so now that you have hp creds that issue should be solved | 03:47 |
wallyworld | davecheney: we devs need to take more responsibility for managing our bugs | 03:47 |
wallyworld | arosales: indeed | 03:47 |
wallyworld | davecheney: in launchpad land, our qa process was much stricter and all branches had to have bugs attached etc and so work couldn't slip through unnoticed | 03:48 |
axw | what level of access to my account does lbox/lpad need? | 03:49 |
davecheney | wallyworld: i'm not sure how to answer that | 03:49 |
davecheney | should we aspire to being more like lp, or less ? | 03:49 |
davecheney | axw: ahh, ther is a trick | 03:49 |
davecheney | read the CONTRIBUTING | 03:49 |
wallyworld | davecheney: in respect to some process type things, more like it | 03:50 |
axw | ta | 03:50 |
davecheney | from memory you need to bzr launchpad-login | 03:50 |
davecheney | or something (it is documented) | 03:50 |
axw | didn't see that, thanks | 03:50 |
davecheney | to generate an oauth key so lbox can fiddle with lp on your behalf | 03:50 |
axw | righto | 03:50 |
davecheney | axw: protip, do it from a fresh terminal so it can fire 'sensible-browser' | 03:51 |
davecheney | wallyworld: what is the tag for goose ? | 03:54 |
davecheney | goose-1.11.4 ? | 03:54 |
wallyworld | juju-1.11.4 | 03:54 |
davecheney | ta | 03:54 |
wallyworld | since it's a part of juju | 03:54 |
davecheney | wallyworld: https://code.launchpad.net/~dave-cheney/+recipe/juju-core | 03:55 |
davecheney | ^ while I was there | 03:55 |
* davecheney goes to rant onthe agenda notes | 03:55 | |
* wallyworld looks | 03:56 | |
davecheney | https://docs.google.com/a/canonical.com/document/d/1JjlwYw8AhF9MxfUJo1SvG3gR9Y5wHox-ToIzUEKoRyg/edit# | 03:56 |
davecheney | ^ looking for the lp # for the two (3) bugs fixed | 03:56 |
wallyworld | davecheney: goose tag looks wrong in the recipe | 03:56 |
wallyworld | bug 1189507 | 03:57 |
_mup_ | Bug #1189507: juju-core duplicates quantum security groups <openstack> <serverstack> <juju-core:Fix Committed by wallyworld> <https://launchpad.net/bugs/1189507> | 03:57 |
wallyworld | bug 1195223 | 03:57 |
_mup_ | Bug #1195223: juju all-machines.log is repetitive and grows unbounded <juju-core:Fix Committed by wallyworld> <https://launchpad.net/bugs/1195223> | 03:57 |
wallyworld | bug 1203658 | 03:57 |
_mup_ | Bug #1203658: bootstrap fails with openstack provider (cannot get endpoint URL without being authenticated) <juju-core:Fix Committed by wallyworld> <https://launchpad.net/bugs/1203658> | 03:57 |
davecheney | thanks mate | 03:58 |
axw | should I be adding "-cr" to get a Rietveld review going? | 04:09 |
axw | er sorry, add to lbox propose | 04:09 |
axw | didn't see it in the CONTRIBUTING doc, but I didn't get any codereview URL from lbox propose | 04:09 |
bigjools | plz to be reviewing https://codereview.appspot.com/11621044 | 04:11 |
bigjools | axw: I just set up a new lbox and didn't need to do that | 04:12 |
davecheney | axw: you _can_ there is a .lbox file in the root of the repo | 04:25 |
davecheney | which I think does that for you | 04:25 |
davecheney | ie, it adjusts some of the defaults | 04:25 |
axw | davecheney: thanks, I saw that later (and thanks bigjools). I think the propose failed due to my lack of permissions in editing bugs | 04:26 |
* bigjools longs for lbox death | 04:27 | |
davecheney | bigjools: we should move to github | 04:27 |
* davecheney ducks and is glad he doesn't live in BNE | 04:27 | |
bigjools | talking of gits... :) | 04:28 |
davecheney | touche | 04:29 |
thumper | pfft | 04:29 |
bigjools | whatever we do, as long as lbox is not involved I'm good | 04:30 |
wallyworld | launchpad is the company standard tool | 04:31 |
wallyworld | we should NEVER has used anything else | 04:31 |
davecheney | wallyworld: you're probably right, but nobody realised you could manage bzr checkouts in your $GOPATH with lightweight checkous | 04:32 |
davecheney | checkouts | 04:32 |
wallyworld | sigh | 04:33 |
* bigjools is using bzr native colo too | 04:33 | |
wallyworld | did anyone ask? | 04:33 |
wallyworld | why assume such things? | 04:33 |
wallyworld | it's not like the bzr folks work for a different company or anything like that | 04:33 |
davecheney | wallyworld: when i joined the team I was told do it this way | 04:33 |
davecheney | you know very well why I wouldn't challenge that | 04:33 |
wallyworld | yeah, i realise. enough said ;-) | 04:34 |
* davecheney thinks of austin, starts to twitch | 04:34 | |
davecheney | no | 04:34 |
davecheney | albama | 04:34 |
davecheney | where the frack did we end up | 04:34 |
bigjools | bzr branch lp:juju then bzr switch -b co:mynewbranch | 04:34 |
davecheney | HOTLANTA! | 04:34 |
bigjools | in other news, the royal lizard hatched | 04:36 |
davecheney | ALL HAIL OUR NEW ROYAL REPTILIAN | 04:37 |
davecheney | i bet all the folks who campaigned for a republic back in 2004 are shitting their pants now | 04:38 |
bigjools | William's heir is coming out | 04:38 |
davecheney | right now ? this second ? | 04:39 |
davecheney | eww | 04:39 |
thumper | bigjools: what? | 04:41 |
thumper | bigjools: lp:juju is the old juju | 04:42 |
bigjools | that's what he said | 04:42 |
thumper | bigjools: also, co: ick | 04:42 |
bigjools | juju-core then, whatever | 04:42 |
bigjools | what's wrong with co:? | 04:42 |
bigjools | works fine for us Reds | 04:42 |
thumper | I just never liked colocated branches | 04:44 |
bigjools | normally I have lightweight checkouts and use a sandbox | 04:45 |
bigjools | and no-tree branches | 04:45 |
bigjools | but go fucked all that over | 04:45 |
davecheney | arosales: ping | 04:47 |
arosales | davecheney, pong | 04:47 |
thumper | bigjools: why, it works for me | 04:48 |
thumper | bigjools: my branch in GOPATH is a lightweight checkout | 04:48 |
thumper | and I use switch and pipelines | 04:48 |
bigjools | thumper: first is that "go get" is broken because I aliased bzr branch to use --no-tree | 04:49 |
bigjools | second is that my no-tree branches are normally in the same dir as go sources, so things get confusing quickly | 04:49 |
bigjools | go projects, I mean | 04:49 |
thumper | bigjools: I have ~/src for repos | 04:50 |
thumper | and branches | 04:50 |
thumper | and just checkouts in GOPATH | 04:50 |
bigjools | is there an easy way of referencing that when using bzr switch without typing the whole path? | 04:50 |
bigjools | if so, I am totally switching back | 04:51 |
thumper | yes | 04:51 |
davecheney | arosales: if you apt-get update && apt-get install juju-core | 04:51 |
thumper | bigjools: so here is what I do | 04:51 |
davecheney | you should have 1.11.4 | 04:51 |
davecheney | tools are uploading now | 04:51 |
davecheney | but precise/amd64 is done | 04:51 |
arosales | davecheney, doing that now | 04:51 |
thumper | bigjools: lets assume we are starting with $GOPATH/src/launchpad.net/juju-core as a lightweight checkout of ~/src/juju-core/trunk | 04:51 |
thumper | bigjools: to make a new branch: bzr switch -b new-work | 04:52 |
thumper | bigjools: to go back to trunk: bzr switch trunk | 04:52 |
thumper | switch looks in the parent dir of the current checkout by default | 04:52 |
thumper | parent dir of the branch of the checkout | 04:52 |
davecheney | thumper: didn't you write a document on this ? | 04:52 |
thumper | davecheney: yes | 04:52 |
thumper | doc/bazaar-usage.txt | 04:52 |
bigjools | thumper: ah parent of the *branch* ! | 04:52 |
bigjools | I thought it was parent of the checkout | 04:52 |
thumper | bigjools: yep | 04:52 |
bigjools | \o/ | 04:52 |
thumper | it may well do that too | 04:53 |
bigjools | that's fucking fantastic | 04:53 |
thumper | and then: bzr push | 04:53 |
thumper | because the push append_policy is linked to the branch, not the checkout | 04:53 |
thumper | automagically goes to the right place in LP | 04:53 |
bigjools | right | 04:53 |
bigjools | have that bit sussed already | 04:53 |
thumper | that is also documented | 04:53 |
* arosales has bootstrapped on hpcloud with 1.11.4 | 04:54 | |
* davecheney high fives wallyworld | 04:55 | |
davecheney | down low ! | 04:55 |
wallyworld | yay | 04:55 |
thumper | too slow | 04:55 |
davecheney | oooh, too slow | 04:55 |
davecheney | jynx! | 04:55 |
bigjools | five side | 04:56 |
arosales | http://pastebin.ubuntu.com/5902956/ thanks for the fast fix and release | 04:56 |
bigjools | scuba dive | 04:56 |
davecheney | ಠ_ಠ| 04:56 |
arosales | why do I only see this error on hpcloud | 04:59 |
arosales | $ juju deploy wordpress | 04:59 |
arosales | error: no instances found | 04:59 |
arosales | if I ran that right after a bootstrap I get that error | 04:59 |
* arosales has to wait for a bit longer after bootstrapping | 04:59 | |
* bigjools believes this can of worms about user expectations was previously discussed | 05:00 | |
davecheney | bigjools: expectatoins == feedback ? | 05:01 |
bigjools | I think it boiled down to either blocking, or providing feedback, yes | 05:01 |
bigjools | I think someone else referenced this lately: https://en.wikipedia.org/wiki/Principle_of_least_astonishment | 05:02 |
wallyworld | thumper: this one should be ok for you to +1 now https://codereview.appspot.com/11659043/ | 05:03 |
axw | where do reviews get mailed to? or do I need to announce them here? | 05:05 |
axw | https://codereview.appspot.com/11419046 | 05:05 |
bigjools | axw: branch mail from launchpad | 05:06 |
axw | bigjools: ok. who do I get to add me to that group? I'd like to be able to see changes as they come in - even if I'm not useful as a reviewer yet | 05:08 |
bigjools | axw: if you are a member of https://launchpad.net/~juju you should get the email by default | 05:08 |
thumper | wallyworld: done | 05:09 |
wallyworld | ty | 05:09 |
axw | I'm not (yet). I guess I need mramm to authorise that | 05:09 |
bigjools | davecheney can auth you | 05:09 |
davecheney | i can fix | 05:09 |
davecheney | shit | 05:10 |
davecheney | this dance | 05:10 |
davecheney | where we add you do groups via a process of eliminatoin | 05:10 |
axw | :) | 05:10 |
davecheney | welcome to your new job, please mind your step | 05:10 |
bigjools | although personally I detest group mail subscriptions | 05:10 |
bigjools | should really encourage people to subscribe to the branch manually and turn off mail for ~juju | 05:11 |
davecheney | axw: which of the page of andrew walkers are you? | 05:11 |
axw | wilkins | 05:11 |
axw | ~axwalk | 05:11 |
davecheney | well, that would explain it | 05:11 |
axw | my username is confusing :) | 05:12 |
bigjools | lol | 05:12 |
davecheney | oh, that should do it | 05:12 |
axw | thanks | 05:14 |
davecheney | axw: re testing that octal change | 05:15 |
davecheney | i guess we don't have anything that is testing that the costs work properly | 05:16 |
davecheney | personally I tihnk all that stuff is shit | 05:16 |
davecheney | and is going to make us cry trying to solve the traveling saleman problem of finding the cheapest instance for the customer | 05:16 |
axw | I don't know it wlel enough to comment yet, though it strikes me as something that should be automatically generated. Not sure if AWS provides a nice data source for that tho | 05:21 |
davecheney | axw: fwreade will know, he did that bit (under duress) | 05:22 |
davecheney | there was some grumlbing that it had to be created by hand | 05:22 |
davecheney | obviously nothing we use that data cared that the cost of a micro was wrong | 05:23 |
davecheney | newsflash, thumper all the 600 phones are sold out | 05:23 |
davecheney | you scored a bargin | 05:24 |
thumper | davecheney: that I did | 05:24 |
thumper | we are at $3.21m now | 05:24 |
thumper | so my indicator says | 05:24 |
davecheney | 10% of the way there | 05:24 |
davecheney | 94% of the time left | 05:24 |
thumper | yeah, not holding my breath | 05:25 |
thumper | would be good though | 05:25 |
davecheney | 27 million dollars is a lot of money let | 05:25 |
davecheney | left | 05:25 |
davecheney | which is sort of a shame | 05:25 |
davecheney | because it casts a shadown on the 3.2mm pledged | 05:25 |
bigjools | ok someone please fix CONTRIBUTING to tell me not to use lbox submit | 05:27 |
thumper | yeah | 05:28 |
thumper | bigjools: sumbit a patch | 05:28 |
thumper | bwahahaha | 05:28 |
bigjools | if I touch that file I'd be sorely tempted to remove references to lbox | 05:29 |
davecheney | bigjools: not til we get inline code reviews in LP | 05:30 |
* bigjools is leaning on lp devs to do that | 05:30 | |
bigjools | heck, could almost do it myself | 05:30 |
bigjools | not sure of the attraction personally, but if it gets rid of lbox ... | 05:31 |
thumper | bigjools: sinzui had ideas too | 05:33 |
thumper | bigjools: perhaps he is easier to ply with alcohol to get it done | 05:33 |
thumper | bigjools: get him right in that bulmer curve | 05:33 |
bigjools | bulmer curve, lol | 05:33 |
thumper | bulmer peak? | 05:34 |
thumper | probably spelled it wrong | 05:34 |
thumper | ballmer peak | 05:34 |
thumper | http://xkcd.com/323/ | 05:34 |
bigjools | Bulmer's is a brand of cider. It sounded perfect. :) | 05:34 |
* thumper likes bulmer's cider | 05:35 | |
thumper | nice on ice on a hot day | 05:35 |
arosales | axw, I saw your comment on 1173093. | 05:45 |
* arosales replied no strong opinion either way | 05:45 | |
axw | arosales: yes, thanks for your reply. going to make the change now | 05:45 |
arosales | me nice to get that one knocked out for our docs | 05:45 |
arosales | s/me/be/ | 05:45 |
arosales | ie "showing how to expose a service" | 05:45 |
axw | just figuring out bzr at the moment | 05:46 |
arosales | missing a fundamental piece on what port is exposed, regression from juju .7 | 05:46 |
arosales | axw, thanks | 05:46 |
davecheney | arosales: pushed all the other tools | 05:47 |
davecheney | are you and jorge happy campers ? | 05:47 |
arosales | davecheney, I have deployed a few services now on hpcloud | 05:48 |
arosales | mysql, wordpress, and juju-gui | 05:48 |
arosales | looking good so far | 05:48 |
arosales | I haven't checked local provider yet, but I think that is unchanged from 1.11.3 | 05:49 |
arosales | davecheney, happier camper | 05:49 |
arosales | thank you wallyworld and davecheney :-) | 05:49 |
arosales | seeing the hardware info is nice too for machines | 05:52 |
arosales | davecheney, fyi agent versions are at 1.11.3, but working just fine for me. | 05:52 |
davecheney | arosales: that can't happen | 05:52 |
davecheney | the cli cannot bootstrap with older tools | 05:53 |
arosales | davecheney, I'll destroy and try again and confirm | 05:54 |
davecheney | -v (always -v) | 05:54 |
arosales | davecheney, did you upload 1.11.4 tools to the hp bucket? | 05:55 |
davecheney | no | 05:56 |
davecheney | how do I do that ? | 05:56 |
davecheney | i don't have permissions to do that | 05:56 |
davecheney | i gues mgz or jam must do it when i'm not watching | 05:56 |
arosales | I do it manually via the console | 05:56 |
arosales | I can add you to that group | 05:56 |
arosales | one sec | 05:56 |
davecheney | so I think it is magic | 05:56 |
arosales | davecheney, your an object store admin for juju-public-tools | 05:58 |
arosales | I mean you were already an admin, no need for me to add you. | 05:58 |
arosales | davecheney, check out arosales@x230:~$ dpkg -l | grep juju-core | 06:00 |
arosales | ii juju-core 1.11.4-1~1514~quantal1 amd64 Juju is devops distilled | 06:00 |
arosales | arosales@x230:~$ juju --debug bootstrap | 06:00 |
arosales | 2013-07-23 05:55:00 INFO juju provider.go:115 environs/openstack: opening environment "hp-go" | 06:00 |
arosales | 2013-07-23 05:55:00 INFO juju provider.go:417 environs/openstack: bootstrapping environment "hp-go" | 06:00 |
arosales | 2013-07-23 05:55:10 INFO juju tools.go:26 environs: reading tools with major version 1 | 06:00 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:49 reading v1.* tools | 06:01 |
arosales | 2013-07-23 05:55:10 INFO juju tools.go:30 environs: falling back to public bucket | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:49 reading v1.* tools | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.10.0-precise-amd64 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.10.0-precise-i386 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.10.0-quantal-amd64 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.10.0-quantal-i386 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.10.0-raring-amd64 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.10.0-raring-i386 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.0-precise-amd64 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.0-precise-i386 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.0-quantal-amd64 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.0-quantal-i386 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.0-raring-amd64 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.0-raring-i386 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.3-precise-amd64 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.3-precise-i386 | 06:01 |
arosales | 2013-07-23 05:55:10 DEBUG juju.environs.tools storage.go:69 found 1.11.3-quantal-a | 06:01 |
davecheney | -v will do | 06:01 |
arosales | ugh sorry | 06:01 |
arosales | buffer wrong | 06:01 |
davecheney | --debug is overkill | 06:01 |
arosales | http://pastebin.ubuntu.com/5903055/ | 06:01 |
arosales | roger | 06:01 |
arosales | davecheney, but I see a 1.11.3 agents running from a 1.11.4 client | 06:06 |
arosales | davecheney, if you have a pointer tothe 1.11.4 tools I can upload those to HP cloud | 06:06 |
davecheney | in related news, https://github.com/andelf/go-curl | 06:12 |
davecheney | bug fixed upstream | 06:12 |
bigjools | davecheney: £$$%^~!"£$^%£ | 06:18 |
mramm2 | axw: hey | 06:19 |
axw | mramm2: hiya | 06:19 |
arosales | davecheney, I see the 1.11.4 tools on aws. I'll download from there and upload to hpcloud | 06:19 |
mramm2 | just getting back from OSCON festivities | 06:19 |
mramm2 | how goes? | 06:19 |
axw | :) | 06:19 |
davecheney | mramm2: nothing to see here, move along | 06:19 |
mramm2 | davecheney: haha | 06:20 |
axw | not bad, people around here have been helping me get acquainted with launchpad and bzr | 06:20 |
mramm2 | sabdfl is happy with things today | 06:20 |
axw | working on a couple of minor bugs | 06:20 |
mramm2 | axw: that's good | 06:20 |
mramm2 | axw: sorry I have not been around much | 06:20 |
axw | mramm2: no worries | 06:21 |
mramm2 | axw: if you need anything let me know | 06:21 |
axw | sure thing | 06:21 |
mramm2 | axw: I know we are all excited to have you on board | 06:21 |
davecheney | mramm2: sure he is, people have promised him 3.2 million dollars | 06:21 |
axw | glad to be here :) wishing I wasn't so inexperienced with these tools though | 06:22 |
mramm2 | thumper and bigjools will not lead you wrong -- well, at least not too much, and not more than I would ;) | 06:22 |
davecheney | arosales: don't worry, there isnt any prior learning assumed | 06:22 |
bigjools | mramm2: *innocent face* | 06:22 |
mramm2 | axw: well there is an adjustment period to any new job | 06:22 |
axw | yup | 06:22 |
mramm2 | bigjools: well, I was being generous ;) | 06:23 |
bigjools | mramm2: happy to return the same generosity :) | 06:23 |
mramm2 | I know -- you are a good dude | 06:23 |
arosales | mramm2, wallyworld and davecheney worked on yet another release ;-) | 06:23 |
mramm2 | arosales: davecheney: great stuff | 06:23 |
arosales | all wallyworld and davecheney | 06:24 |
mramm2 | I know mims and jorge will be grateful | 06:24 |
mramm2 | arosales: I know, you and I are just managers -- nothing to see here, please move along | 06:24 |
* arosales mostly getting in the way | 06:25 | |
mramm2 | davecheney: it's not just the phone stuff -- though that is awesome. sabdifl is also very happy with the local provider and the gui updates -- he is feeling the juju love | 06:25 |
mramm2 | and as we all know "juju is...." | 06:25 |
mramm2 | "fucking amazing" | 06:26 |
* davecheney basks in sabdfl's munificence | 06:26 | |
mramm2 | haha | 06:26 |
davecheney | do you want a happy god, or an angry god ? | 06:26 |
mramm2 | davecheney: well, the key is having a competent god ;) | 06:27 |
mramm2 | and I am happy to work for one of those ;) | 06:27 |
mramm2 | lots of winky faces from me this evening -- perhaps I have had one too many beers! | 06:27 |
davecheney | has anyone been able to get to Gustavo's ubuntu-edge.info ? | 06:27 |
mramm2 | I got to it a few min ago | 06:27 |
bigjools | yes | 06:27 |
axw | yep | 06:27 |
mramm2 | on my walk back to the hotel | 06:27 |
* davecheney tries his phone | 06:28 | |
davecheney | stupid internet | 06:28 |
mramm2 | if only you had an ubuntu phone -- it would all work perfectly! | 06:28 |
mramm2 | we had a "ubuntu in production" bof this evening, and met lots of people using ubuntu in interesting places | 06:29 |
mramm2 | everything from gas-station management to top 100 web sites | 06:29 |
mramm2 | we asked everybody what they wanted from us, and they all told us that we were perfect | 06:30 |
mramm2 | I don't believe it, but it was nice to hear ;) | 06:30 |
arosales | mramm2, remind me to tell hazmat that deployer configs need quotes around "no" and "yes" values or deployer chokes | 06:31 |
mramm2 | arosales: I will try to remind you ;) | 06:32 |
mramm2 | arosales: I will likely be busy for the next couple of days -- so you might also want to make a note of it ;) | 06:32 |
arosales | perhaps that is a juju-gui export though | 06:33 |
arosales | mramm2, roger. | 06:33 |
arosales | mramm2, ack | 06:33 |
mramm2 | arosales: the gui is looking great, got good reviews today | 06:33 |
mramm2 | and jorge castro is in seems to have a geek crush on thumper -- aka: the devops bunny | 06:34 |
mramm2 | because of the local provider, which also got good reviews from hazmat | 06:34 |
arosales | mramm2, ha | 06:35 |
* arosales glad to see it went full screen by default | 06:35 | |
mramm2 | not sure if thumper will be able to live that one down | 06:35 |
mramm2 | oh, and BTW, if any of you wants to go to the openstack summit -- we need some good juju related talk proposals | 06:39 |
mramm2 | shoot them my way so that I can coordinate | 06:39 |
mramm2 | next openstack summit is in hong kong, so those of you on that side of the world have the travel advantage ;) | 06:41 |
davecheney | mramm2: i hear you loud and clear | 06:42 |
mramm2 | well, I have an early morning, talk to you all later! | 06:45 |
axw | night | 06:47 |
arosales | axw, night and welcome | 06:47 |
axw | arosales: I'm not going, but if you are going also, good night :) | 06:48 |
axw | and thanks | 06:48 |
rogpeppe1 | mornin' all | 07:12 |
axw | morning | 07:13 |
rvba | Morning guys… anyone up for a tiny review? https://codereview.appspot.com/11578043/ | 07:13 |
davecheney | jamespage: mgz wallyworld just doing the tarball for 1.11.4 | 07:15 |
wallyworld | awesome | 07:15 |
davecheney | 1.11.4 _does_ include gwacl | 07:15 |
davecheney | but does _not_ include go-curl | 07:15 |
davecheney | and does not require libcurl-dev | 07:16 |
wallyworld | yay | 07:16 |
jamespage | davecheney, ack | 07:19 |
davecheney | ok, that is it for me | 07:25 |
davecheney | releases are released | 07:25 |
davecheney | uploads are uploaded | 07:26 |
dimitern | \o/ | 07:26 |
davecheney | buttons pressed | 07:26 |
davecheney | ticks boxed | 07:26 |
dimitern | machiner api-ed | 07:26 |
davecheney | openstack, stacked | 07:27 |
dimitern | ubuntu edged ;) | 07:27 |
* davecheney rimshot | 07:28 | |
dimitern | man, more than $3.2M pledged in less than a day | 07:28 |
wallyworld | dimitern: i won't make the meeting. i'm about to go play soccer | 07:36 |
dimitern | wallyworld: ok | 07:39 |
wallyworld | i'm sure you won't miss me | 07:39 |
dimitern | :) sure | 07:40 |
dimitern | :P | 07:41 |
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: rogpeppe | Bugs: 5 Critical, 76 High - https://bugs.launchpad.net/juju-core/ | ||
axw | what LeanKit board(s) are being used for juju-core? i.e. what boards should I request access to? | 08:09 |
rogpeppe1 | axw: https://canonical.leankit.com/Boards/View/103148069 | 08:11 |
axw | rogpeppe1: thanks | 08:11 |
noodles775 | Hey there. I tried to install from source yesterday, but it failed with "No package 'libcurl' found. I read above that libcurl-dev is no longer required with todays release, so just tried the source again - which doesn't error, but doesn't result in $GOPATH/bin/juju either: http://paste.ubuntu.com/5903326/ | 08:20 |
noodles775 | Hrm - nm. It looks like I'm confused by the output of the second "go install" there, the exit status is still 1. | 08:22 |
* noodles775 installs libcurl-dev. | 08:22 | |
rvba | noodles775: go-curl (and thus libcurl) should not be a dependency any more… are you installing from fresh or upgrading? | 08:25 |
rvba | noodles775: could you make sure gwacl is up to date by running: go get -u launchpad.net/gwacl | 08:27 |
noodles775 | rvba: thanks. That returns no output. Let me uninstall libcurl-dev again... | 08:29 |
dimitern | rogpeppe1: so you're doing the upgrader, right? | 08:31 |
rogpeppe1 | dimitern: yes | 08:31 |
noodles775 | rvba: Yep - that compiles now: http://paste.ubuntu.com/5903360/ Thanks! | 08:31 |
dimitern | rogpeppe1: i'll pick up the deployer then | 08:31 |
rogpeppe1 | dimitern: cool | 08:31 |
dimitern | rogpeppe1: it needs a StringsWorker, following the same model like the NotifyWorker | 08:32 |
rvba | noodles775: welcome :) | 08:32 |
rogpeppe1 | dimitern: are there going to be any others using StringsWorker? | 08:32 |
dimitern | rogpeppe1: uniter perhaps? | 08:32 |
rogpeppe1 | dimitern: i very much doubt it | 08:33 |
dimitern | rogpeppe1: firewaller? | 08:33 |
rogpeppe1 | dimitern: can't we just use the existing logic rather than refactoring everything? | 08:34 |
dimitern | rogpeppe1: you don't think introducing the NotifyWorker was a good thing? | 08:34 |
rogpeppe1 | dimitern: it's a good thing if it's used more than once | 08:34 |
dimitern | rogpeppe1: so, that's my answer to the above :) | 08:35 |
rogpeppe1 | dimitern: otherwise it's just unnecessary abstraction | 08:35 |
rogpeppe1 | dimitern: the firewaller won't be able to use StringsWorker AFAICS | 08:35 |
dimitern | rogpeppe1: it uses an environconfigwatcher and environmachineswatcher, the second one is a stringswatcher | 08:36 |
rogpeppe1 | dimitern: how are you going to structure the StringsWorker so that it allows more entries in the core select loop? | 08:36 |
dimitern | rogpeppe1: more entries? | 08:36 |
dimitern | rogpeppe1: it should be simple | 08:37 |
rogpeppe1 | dimitern: currently the firewaller listens on 6 things in its select | 08:37 |
dimitern | rogpeppe1: couldn't we use several workers and put them together, rather than having a single loop? | 08:38 |
rogpeppe1 | dimitern: the NotifyWorker pattern is to listen on 2 things (a watcher and the tomb) and make a callback when one of them happens | 08:38 |
rogpeppe1 | dimitern: i don't see that that would gain us anything but obfuscation | 08:38 |
rogpeppe1 | dimitern: the firewaller control flow is current quite nice and obvious | 08:38 |
rogpeppe1 | dimitern: i'd be happy to keep it that way | 08:38 |
dimitern | rogpeppe1: ok then, won't do it for now; we can always refactor it later if needed | 08:38 |
rogpeppe1 | dimitern: thanks | 08:39 |
dimitern | rogpeppe1: fairly trivial https://codereview.appspot.com/11711043/ | 08:54 |
rogpeppe1 | dimitern: i thought i'd done that... | 08:55 |
rogpeppe1 | dimitern: yeah, it's at https://codereview.appspot.com/11586043/ | 08:55 |
rogpeppe1 | dimitern: i'm still waiting for a second review | 08:55 |
rogpeppe1 | dimitern: you even reviewed the code :-) | 08:56 |
dimitern | rogpeppe1: oh :) | 08:56 |
dimitern | TheMue: can you review this please? ^^ | 08:57 |
dimitern | rogpeppe1: i need it to land to continue on the deployer | 08:57 |
rogpeppe1 | dimitern: well, i'll need another review then... | 08:57 |
rogpeppe1 | anyone? | 08:57 |
dimitern | mgz: ? | 08:58 |
TheMue | dimitern: sure | 08:59 |
dimitern | TheMue: thanks | 08:59 |
dimitern | rogpeppe1: I'll reject mine then | 08:59 |
TheMue | rogpeppe1: you've got a +1 | 09:03 |
rogpeppe1 | TheMue: thanks | 09:03 |
dimitern | TheMue: sorry, not that | 09:03 |
dimitern | TheMue: https://codereview.appspot.com/11586043/ this | 09:03 |
* TheMue currently build a test-env for autosync tests | 09:03 | |
dimitern | TheMue: the one you reviewed I closed, because it duplicates rogpeppe1's | 09:04 |
TheMue | dimitern: don | 09:04 |
TheMue | done | 09:04 |
dimitern | TheMue: tyvm | 09:04 |
TheMue | dimitern: seen it after lgtm'ed it ;) | 09:04 |
dimitern | :) np | 09:04 |
dimitern | rogpeppe1: good to go then | 09:05 |
rogpeppe1 | dimitern: ok, it's approved | 09:05 |
dimitern | rogpeppe1: cheers | 09:06 |
jtv2 | mgz: your oldest pending branch looks about ready to land... | 09:11 |
axw | jtv2, rogpeppe1: thanks (yes I did confirm pricing on AWS). Do I do "lbox submit" now? | 09:41 |
rogpeppe1 | axw: no; we don't use that any more | 09:41 |
rogpeppe1 | axw: you go to the merge proposal | 09:41 |
axw | claim review? | 09:41 |
rogpeppe1 | axw: copy and paste the mp description into the commit message | 09:41 |
rogpeppe1 | axw: then mark the mp as approved | 09:42 |
axw | ok | 09:42 |
jtv2 | No need to claim the review. | 09:42 |
jtv2 | Although I always add an Approved vote listing who approved in Rietveld, but that's probably just because I'm used to Tarmac requiring an approval vote. | 09:42 |
rvba | rogpeppe1: if you have time for a tiny review: https://codereview.appspot.com/11578043/. William had a look at it on Friday but asked me a question (I replied) and didn't give me a formal LGTM. | 10:03 |
rogpeppe1 | rvba: LGTM | 10:06 |
rvba | Thanks. | 10:07 |
axw | adios | 10:09 |
rogpeppe1 | i'd appreciate a couple of reviews of this please: https://codereview.appspot.com/11680043/ | 10:18 |
mgz | looking | 10:22 |
TheMue | rogpeppe1: done | 10:22 |
rogpeppe1 | TheMue: thanks | 10:22 |
mgz | rogpeppe1: so if I understood correctly yesterday, we have an older copy of of this from goamz, and this basically takes the newer version? | 10:23 |
rogpeppe1 | mgz: not quite. goamz copied the juju-core version, then updated it. this CL copies back that updated version. | 10:24 |
mgz | aha, fun. | 10:24 |
mgz | approved. | 10:26 |
rogpeppe1 | mgz: ta | 10:26 |
mgz | rogpeppe1: have you got opinions on https://coderview.appspot.com/11679044 specifically the status output format? | 10:58 |
rogpeppe1 | mgz: i get a 404 for that link | 10:58 |
mgz | +e | 10:59 |
mgz | codereview | 10:59 |
rogpeppe1 | mgz: ha, it's funny that coderview is actually a site | 11:00 |
rogpeppe1 | mgz: hmm, i think we should probably keep the same format as the python | 11:02 |
rogpeppe1 | mgz: it's not as if there's a strong convention for tcp:45 | 11:03 |
rogpeppe1 | mgz: perhaps we should just change state.Port.String to print tcp/45 instead | 11:03 |
rogpeppe1 | mgz: i can't immediately think of anything that would break | 11:03 |
mgz | that would be my prefered option if we can do it without breakage | 11:03 |
* TheMue => lunch | 11:15 | |
rogpeppe1 | mgz: you've got another review: https://codereview.appspot.com/11548044/ | 11:26 |
mgz | ta, I should unheadinsand and land | 11:27 |
mgz | standup all? | 11:33 |
=== rogpeppe1 is now known as rogpeppe | ||
rogpeppe | TheMue: you have a review | 11:54 |
TheMue | rogpeppe: thanks | 11:54 |
dimitern | rogpeppe, TheMue: https://codereview.appspot.com/11713043 | 11:55 |
dimitern | TheMue: thanks | 12:06 |
TheMue | dimitern: yw | 12:07 |
rogpeppe | dimitern: you've got a review: https://codereview.appspot.com/11713043/ | 12:13 |
dimitern | rogpeppe: thanks | 12:14 |
dimitern | rogpeppe: the server always returns results and they are exactly the same number as the passed args | 12:16 |
rogpeppe | dimitern: we don't want to panic if the client's talking to a dodgy server though, do we? | 12:16 |
rogpeppe | dimitern: otherwise why bother checking the length at all? | 12:16 |
rogpeppe | dimitern: (as you do in the original if statement) | 12:17 |
=== gary_pos` is now known as gary_poster | ||
dimitern | rogpeppe: hmm | 12:18 |
dimitern | rogpeppe: ok, but not sure about the return "", fmt.Errorf("no results found") | 12:19 |
rogpeppe | dimitern: well, yeah, choose some appropriate error msg | 12:19 |
dimitern | rogpeppe: should there be any? the AssignedMachineId from state doesn't behave like this | 12:19 |
rogpeppe | dimitern: that method isn't a bulk method | 12:20 |
rogpeppe | dimitern: if the server returns no results and no error, then there's something gone wrong | 12:20 |
dimitern | rogpeppe: and the one in the client-side api isn't either | 12:20 |
rogpeppe | dimitern: but the server one is | 12:20 |
rogpeppe | dimitern: so we need to decide what we want to do if the server returns no results and no error | 12:21 |
dimitern | rogpeppe: how about ErrUnexpected "unexpected server response" ? | 12:21 |
rogpeppe | dimitern: i'd prefer to be a bit more explicit in the error message. | 12:21 |
dimitern | rogpeppe: fwiw this has to change for all other api client-side methods like this | 12:21 |
rogpeppe | dimitern: how about ErrNoResults "no results found in server response" | 12:22 |
dimitern | rogpeppe: sounds good | 12:22 |
dimitern | rogpeppe: and I'll add a TODO to change all other methods like that | 12:23 |
rogpeppe | dimitern: well various places already handle it | 12:23 |
rogpeppe | dimitern: e.g. deployer.State.unitLife | 12:23 |
dimitern | rogpeppe: like where? | 12:23 |
rogpeppe | return "", fmt.Errorf("expected one result, got %d", len(result.Results)) | 12:23 |
dimitern | rogpeppe: that's different | 12:23 |
rogpeppe | dimitern: i think it's exactly the same, isn't it? | 12:23 |
dimitern | rogpeppe: Life() returns no error, unitLife is used for Refresh() | 12:24 |
rogpeppe | dimitern: it handles exactly the same case | 12:24 |
rogpeppe | dimitern: it's handling a server call that may return no results and no errors in a robust way | 12:24 |
rogpeppe | dimitern: as i'm suggesting you do | 12:24 |
dimitern | rogpeppe: ok, i'll add the error and add todos as appropriate | 12:24 |
rogpeppe | dimitern: i don't see anywhere else that doesn't do it right | 12:26 |
rogpeppe | dimitern: most places return "expected one result..." | 12:26 |
dimitern | rogpeppe: ok, so why not instead of adding this extra error nobody else will probably use, change it so it works like the other calls, i.e. "expected one result..." | 12:27 |
rogpeppe | dimitern: sgtm | 12:27 |
dimitern | rogpeppe: good point about IsResponsibleForUnit(), but how should I call the client-side unit call? | 12:30 |
dimitern | rogpeppe: unit.IsResponsible? | 12:31 |
rogpeppe | dimitern: hmm | 12:31 |
rogpeppe | dimitern: unit.AmResponsible ? | 12:32 |
dimitern | rogpeppe: IsAgentResponsible? | 12:32 |
dimitern | rogpeppe: CanDeploy? | 12:32 |
rogpeppe | dimitern: i like CanDeploy | 12:32 |
dimitern | rogpeppe: ok | 12:32 |
rogpeppe | dimitern: then the server call becomes CanDeployUnits | 12:33 |
rogpeppe | dimitern: or perhaps just CanDeploy actually | 12:33 |
dimitern | rogpeppe: sgtm | 12:34 |
TheMue | strange, the whole call chain a tools.ErrNoTools is returned. but when I compare the this error the "if" fails :/ | 12:41 |
dimitern | rogpeppe: updated https://codereview.appspot.com/11713043 | 12:46 |
* dimitern bbiab | 12:47 | |
rogpeppe | dimitern: could you remind me why the original code checked the assigned machine id, please? | 13:01 |
rogpeppe | dimitern: after all, we've just received the unit name from a watcher which is presumably watching only units for this particular machine | 13:02 |
* rogpeppe is called to lunch | 13:02 | |
dimitern | rogpeppe: because we need to know if we are responsible for deploying that unit | 13:05 |
dimitern | rogpeppe: we're calling assignedmachineid to see if the unit is assigned for deployment, and yes, we only watch units of one machine (the deployer's) | 13:06 |
rogpeppe | dimitern: so what's a scenario where the deployer sees a unit that it's not responsible for? | 13:30 |
rogpeppe | dimitern: isn't the deployer now responsible for deploying subordinate units too? (or is that something that is still in the murky future?) | 13:31 |
dimitern | rogpeppe: if it isn't responsible for a unit it won't try deploying it | 13:43 |
rogpeppe | dimitern: yes, but why would it see a unit it isn't responsible for in the watcher results? | 13:43 |
dimitern | rogpeppe: because the unit is not assigned yet | 13:44 |
rogpeppe | dimitern: doesn't the watcher watch only assigned units? | 13:44 |
dimitern | rogpeppe: hmm | 13:45 |
dimitern | rogpeppe: good question, have to check | 13:45 |
dimitern | rogpeppe: it watches all principal units of a machine (assigned ones), and also all subs of these principals (which might not be assigned yet) | 13:46 |
rogpeppe | dimitern: i don't think a sub can be unassigned | 13:46 |
rogpeppe | dimitern: it's assigned by virtue of being a sub of an assigned principal | 13:47 |
dimitern | rogpeppe: yeah, that's right | 13:47 |
rogpeppe | dimitern: i *think* i see the issue | 13:48 |
dimitern | rogpeppe: but not sure assignedmachineid returns != "" for subs | 13:48 |
rogpeppe | dimitern: i think it probably would | 13:48 |
rogpeppe | dimitern: but anyway... | 13:48 |
rogpeppe | dimitern: it seems like the watcher doesn't tell you if it's telling you about a unit that's assigned or a unit that's been unassigned | 13:48 |
dimitern | rogpeppe: it returns the principal's machine id (if assigned) | 13:49 |
rogpeppe | dimitern: it just tells you about changes | 13:49 |
dimitern | rogpeppe: that seems right, yes | 13:49 |
rogpeppe | dimitern: even though it actually knows what the changes are (added or removed) | 13:49 |
rogpeppe | dimitern: this seems a bit odd, because you can't understand what the watcher's telling you without querying the state, but by that time it might be out of date. | 13:50 |
rogpeppe | dimitern: perhaps this was done as part of the recent StringsWatcher changes | 13:50 |
dimitern | rogpeppe: how could it be out of date, when you're just checking that? | 13:51 |
dimitern | rogpeppe: i don't get you, sorry | 13:51 |
rogpeppe | dimitern: well, it might be telling you that it's been just unassigned, but by the time you get around to checking, it's been assigned again. | 13:52 |
rogpeppe | dimitern: i guess it doesn't matter | 13:52 |
rogpeppe | dimitern: it just seems a bit odd, and it's the only reason for this API call you're doing | 13:54 |
rogpeppe | dimitern: if the watcher specified which units were assigned and which units were unassigned, we wouldn't need it | 13:54 |
dimitern | rogpeppe: perhaps, but we still need to check if it's assigned between we read from the watcher changes chan and we're about to deploy, based on that decision, won't we? | 13:55 |
rogpeppe | dimitern: not really, because that only narrows the race window AFAICS | 13:55 |
rogpeppe | dimitern: anyway, i now think i understand | 13:56 |
dimitern | rogpeppe: :) | 13:56 |
dimitern | rogpeppe: it's a bit hairy i guess | 13:57 |
rogpeppe | dimitern: there's a minor security issue which is that this call allows any machine agent to probe for the existence of any unit | 13:57 |
dimitern | rogpeppe: how come? | 13:57 |
rogpeppe | dimitern: issue the call and see what result you get | 13:57 |
rogpeppe | dimitern: if you get ErrNotFound, the unit doesn't exist; if you get ErrPerm, it does | 13:57 |
dimitern | rogpeppe: you won't get err not found | 13:58 |
rogpeppe | dimitern: no? | 13:58 |
dimitern | rogpeppe: where? | 13:58 |
rogpeppe | dimitern: from CanDeploy | 13:58 |
rogpeppe | dimitern: pass it a unit name that doesn't exist | 13:58 |
rogpeppe | dimitern: d.st.Unit will return ErrNotFound (or similar) | 13:59 |
dimitern | rogpeppe: yeah, and you get result!=true, so you get errperm | 13:59 |
rogpeppe | dimitern: ah, i see | 13:59 |
dimitern | rogpeppe: ah, you mean client-side? | 13:59 |
rogpeppe | dimitern: no, i'd misread your logic, sorry | 14:00 |
dimitern | rogpeppe: you'll get nil always, but any next call will return errperm | 14:00 |
rogpeppe | dimitern: i think that code is odd though - it's odd that you can't get false without an error | 14:00 |
dimitern | rogpeppe: not from the server | 14:01 |
dimitern | rogpeppe: it always returns errPerm on false | 14:01 |
rogpeppe | dimitern: yeah, that's weird | 14:02 |
rogpeppe | dimitern: how about never returning an error. | 14:02 |
rogpeppe | ? | 14:02 |
dimitern | rogpeppe: why? | 14:02 |
rogpeppe | dimitern: because there's no information content in the error | 14:02 |
mgz | rogpeppe: ah, I'll have to look at godoc example code stuff, that's neat | 14:02 |
rogpeppe | dimitern: result==false => err==ErrPerm; result==true => err==nil | 14:03 |
rogpeppe | mgz: yeah, it works quite nicely. | 14:03 |
rogpeppe | mgz: it's nicer when you can actually have some output - then gotest will run the code and check the output for you | 14:03 |
dimitern | rogpeppe: let's not hide details like that | 14:03 |
rogpeppe | dimitern: i'm not sure what you mean | 14:03 |
rogpeppe | dimitern: i was just describing the current behaviour | 14:04 |
dimitern | rogpeppe: I prefer not to do that | 14:05 |
rogpeppe | dimitern: better might be to return an error if we get an non-not-found error from Unit; otherwise return true iff the assigned machine matches | 14:06 |
dimitern | rogpeppe: that might work | 14:06 |
dimitern | rogpeppe: ok, sounds good | 14:06 |
rogpeppe | dimitern: cool | 14:06 |
rogpeppe | dimitern: then you won't be discarding the error always like you are currently | 14:07 |
dimitern | rogpeppe: agreed | 14:08 |
rogpeppe | dimitern: i've made a couple more comments | 14:10 |
dimitern | rogpeppe: thanks | 14:10 |
rogpeppe | mgz: golang.org/pkg/testing documents the example code conventions FYI | 14:11 |
rogpeppe | mgz: (not the most intuitive place!) | 14:11 |
mgz | yeah, I was surprised you added it to the test code, but it makes some sense | 14:11 |
dimitern | rogpeppe: wait a moment | 14:16 |
dimitern | rogpeppe: if we get notfounderr from st.Unit, we still don't know whether we're talking to an authorized entity | 14:16 |
dimitern | rogpeppe: that's why i think we still should return errperm, like in the other facades | 14:17 |
rogpeppe | dimitern: i don't think it should matter | 14:17 |
dimitern | rogpeppe: once we have the unit and it's assigned, we can call authOwner, etc. | 14:17 |
rogpeppe | dimitern: the question the API call is answering is: "can i deploy this unit?" | 14:17 |
dimitern | rogpeppe: it matters, because returning not found is exactly what you described as "minor security risk" | 14:17 |
rogpeppe | dimitern: i don't think it matters if the answer is "false" for units you can't deploy | 14:17 |
rogpeppe | dimitern: i'm not suggesting that we return not found | 14:18 |
rogpeppe | dimitern: i'm suggesting we return false instead of ErrNotFound | 14:18 |
dimitern | rogpeppe: the question, as always, implicitly like for every API call is, "can I do this at all?" | 14:18 |
rogpeppe | dimitern: you can ask the question | 14:18 |
rogpeppe | dimitern: that's not an error | 14:18 |
dimitern | rogpeppe: it's an errperm if we don't know if you're supposed to ask / know that | 14:19 |
rogpeppe | dimitern: but you can't find out anything more than "yes you can deploy this unit" or "no you cannot deploy this unit or it doesn't exist" | 14:19 |
dimitern | rogpeppe: isn't that the whole point of the call? | 14:19 |
rogpeppe | dimitern: the point of the call is to ask if we can deploy a given unit | 14:20 |
dimitern | rogpeppe: it's a yes/no question | 14:20 |
rogpeppe | dimitern: why should we return ErrPerm if the answer is false? | 14:20 |
rogpeppe | dimitern: i can't see any possible security hole | 14:20 |
rogpeppe | dimitern: or information leak | 14:21 |
rogpeppe | dimitern: and it just makes the client code more complex | 14:21 |
rogpeppe | dimitern: because it then has to explicitly check for ErrPerm | 14:21 |
dimitern | rogpeppe: if it's false, and we're authorized, i agree it should be false, nil | 14:21 |
rogpeppe | dimitern: ha ha! | 14:21 |
rogpeppe | dimitern: it can't be false if we're authorized! | 14:22 |
dimitern | rogpeppe: not now, but it will be :) | 14:22 |
rogpeppe | dimitern: how so? | 14:22 |
dimitern | rogpeppe: i'll change it to return false, nil if we're authorized and cannot deploy it | 14:22 |
dimitern | rogpeppe: is that what you want? | 14:22 |
rogpeppe | dimitern: i'm not sure. if i'm the machine agent, the watcher tells me about a unit, the unit gets unassigned, i ask whether i can deploy the unit, then i don't think i should get ErrPerm. it should just return false. | 14:23 |
rogpeppe | dimitern: i can't see any security issue with doing that | 14:24 |
dimitern | rogpeppe: so the cases are: 1) false, errperm (not found, etc.), 2) true, nil (assignedmachineid == entity.tag), 3) any other case (currently) is the same as 1) | 14:25 |
rogpeppe | dimitern: and it means that the machine agent doesn't need to say "oh, ErrPerm? that actually means I can't deploy the unit" | 14:25 |
dimitern | rogpeppe: and we can change 3) false, nil to mean (no error, and you cannot deploy it) | 14:25 |
dimitern | rogpeppe: but I don't see how we can do that | 14:25 |
dimitern | rogpeppe: and still have permission checks | 14:26 |
dimitern | rogpeppe: i.e. not leak unnecessary info | 14:26 |
rogpeppe | dimitern: i suggest: 1) false, some-non-not-found-error; 2) true, nil; 3) false, nil | 14:26 |
rogpeppe | dimitern: 1) happens when we get an error talking to the state | 14:26 |
rogpeppe | dimitern: 2) happens if you can deploy the unit | 14:27 |
rogpeppe | dimitern: 3) happens if the unit isn't assigned to the given machine agent or doesn't exist. | 14:27 |
dimitern | rogpeppe: 1) the error has to be errperm | 14:27 |
rogpeppe | dimitern: why? | 14:27 |
dimitern | rogpeppe: and 3) you cannot know which machine agent we're talking about | 14:27 |
dimitern | rogpeppe: at server-side | 14:28 |
rogpeppe | dimitern: huh? we know the authenticated machine agent. that's how we're checking AuthOwner, right? | 14:28 |
dimitern | rogpeppe: I mean, we know the authed entity, but not how the unit relates to it, until we see assigned machine id | 14:28 |
rogpeppe | dimitern: yes, and? | 14:28 |
dimitern | rogpeppe: oh boy | 14:29 |
rogpeppe | dimitern: i don't (currently) see how a non-not-found error can leak info | 14:29 |
dimitern | rogpeppe: I realized I have an error there | 14:29 |
dimitern | rogpeppe: it has to be authOwner(entity.Tag) && entity.Tag == assignedMachineId to be true, nil | 14:29 |
dimitern | rogpeppe: that's for 2) | 14:30 |
dimitern | rogpeppe: and for 1), I already agreed non-not-found errors should be returned as it, otherwise - errperm | 14:30 |
rogpeppe | dimitern: how can authOwner(entity.Tag) ever be true? | 14:31 |
rogpeppe | dimitern: given that the entity is a unit tag | 14:31 |
dimitern | rogpeppe: entity.Tag => authorized entity's tag | 14:32 |
dimitern | rogpeppe: that's what I meant | 14:32 |
rogpeppe | dimitern: what's wrong with authOwner(assignedMachineId) as i suggested? | 14:32 |
rogpeppe | dimitern: for true, nil | 14:33 |
dimitern | rogpeppe: ah, right, sorry - too many variables :) | 14:33 |
dimitern | rogpeppe: i agree with that | 14:33 |
rogpeppe | dimitern: so, i can't see a time when it's appropriate to return ErrPerm currently | 14:33 |
dimitern | rogpeppe: but that's what I did anyway | 14:34 |
dimitern | rogpeppe: machineId, er := unit.AssignedMachineId(); machineTag := state.MachineTag(machineId); AuthOwner(machineTag), no? | 14:34 |
rogpeppe | dimitern: istm that by returning ErrPerm there, we're trying to say "you can't even ask that question", but we're answering it anyway | 14:35 |
dimitern | rogpeppe: in 2 cases at least | 14:35 |
rogpeppe | dimitern: yes | 14:35 |
rogpeppe | AuthOwner(state.MachineTag(machineId)) i think i suggested in my comment | 14:35 |
dimitern | rogpeppe: 1) non-not-found error in st.Unit() -> errPerm, 2) err != nil in assignedmachineId -> errPerm, i think | 14:35 |
dimitern | rogpeppe: sorry, s/non-not/not/ | 14:37 |
dimitern | rogpeppe: and 3) assignedMachineId == "" -> errPerm, i think as well | 14:37 |
dimitern | rogpeppe: i.e. until we know the assignedMachineId != "" and err == nil, the error is errPerm, after that it's errPerm only when !authOwner(assignedMachineTag) | 14:39 |
dimitern | rogpeppe: the truth table grew a lot :) | 14:40 |
rogpeppe | dimitern: sorry, had to take a parcel at the door | 14:40 |
rogpeppe | dimitern: i'd like it if a machine agent cannot get an error in the normal course of asking the CanDeploy question | 14:41 |
dimitern | rogpeppe: should I summarize it again? (i got confused myself) - 1) err = st.Unit(); err != notFound -> err, else -> errPerm, 2) assignedId, err = u..(); err != nil -> errPerm (i think), else assignedId == "", definitely errPerm, 3) !authOwner(assignedTag) -> errPerm, 4) else - true, nil | 14:42 |
dimitern | rogpeppe: how about that? | 14:43 |
rogpeppe | [15:41:44] <rogpeppe> dimitern: i'd like it if a machine agent cannot get an error in the normal course of asking the CanDeploy question | 14:44 |
rogpeppe | dimitern: i don't see how your suggestion addresses that | 14:45 |
dimitern | rogpeppe: how is that not leaking information? | 14:45 |
rogpeppe | dimitern: i don't think it should be a permission-denied error for a deployer to ask about a unit it's just been told about | 14:45 |
rogpeppe | dimitern: what info does it leak? | 14:45 |
dimitern | rogpeppe: suppose I'm MA for m0 and asking can I deploy u1, which is assigned to m1 - shouldn't that be errPerm ? | 14:46 |
rogpeppe | dimitern: if i just get the answer "false", what information have i gathered? | 14:46 |
dimitern | rogpeppe: that perhaps there indeed is a u1 and I'm not allowed to touch it | 14:47 |
dimitern | rogpeppe: whereas errPerm means you can't touch u1 anyway, so the question is irrelevent | 14:48 |
rogpeppe | dimitern: that's no information. it doesn't tell you anything about the existence or non-existence of u1 | 14:48 |
rogpeppe | dimitern: "perhaps" | 14:48 |
dimitern | rogpeppe: no perhaps, sorry, if there isn't a u1 you'll get errPerm anyway earlier | 14:49 |
rogpeppe | dimitern: i suggest you should get false in that case | 14:49 |
dimitern | rogpeppe: it'll be not found -> errPerm | 14:49 |
dimitern | rogpeppe: so if u1 exists, I'll get false, nil, and if it doesn't i'll get false, nil, is that what you're saying? | 14:50 |
rogpeppe | dimitern: exactly | 14:50 |
dimitern | rogpeppe: sorry I mean u1 doesn't exist -> false, errPErm | 14:50 |
rogpeppe | dimitern: yeah, i suggest false, nil there | 14:50 |
dimitern | rogpeppe: the thing is the existence of u1 changes the result | 14:50 |
rogpeppe | dimitern: how so? | 14:51 |
rogpeppe | dimitern: you get false, nil in both cases | 14:51 |
dimitern | rogpeppe: u1 doesn't exist (not found -> false, errPerm, earlier), u1 exists -> false, nil | 14:51 |
rogpeppe | dimitern: for the not found case, i suggest (false, nil) as i said earlier | 14:52 |
dimitern | rogpeppe: why? | 14:52 |
dimitern | rogpeppe: that's different from all other api calls so far | 14:52 |
rogpeppe | dimitern: because it's perfectly ok for a machine agent to ask about a unit it's just been told about but happens to have been deleted. | 14:52 |
dimitern | rogpeppe: all others return errPerm on not found | 14:52 |
rogpeppe | dimitern: i think this *is* a bit different | 14:52 |
dimitern | rogpeppe: don't assume what MA's been told | 14:52 |
dimitern | rogpeppe: we're talking about the subtle case of a rouge agent here | 14:53 |
rogpeppe | dimitern: i'm not making that assumption | 14:53 |
dimitern | rogpeppe: what's stops an agent from asking for an arbitrary unit | 14:53 |
rogpeppe | dimitern: it can ask for an arbitrary unit, but it can't find out any information about it at all | 14:53 |
dimitern | rogpeppe: and getting false, nil means either (unit exists, but you cannot deploy it) or (unit does not exist) | 14:54 |
rogpeppe | dimitern: unless that unit has been assigned to it | 14:54 |
rogpeppe | dimitern: yeah, that seems good to me | 14:54 |
rogpeppe | dimitern: you're allowed to ask the question | 14:54 |
dimitern | rogpeppe: whereas all other calls return errPerm when unit doesn't exist and may return errPerm down the line if you're not authorized | 14:54 |
rogpeppe | dimitern: that's usually because it's an error to try to talk to an entity that doesn't exist | 14:55 |
dimitern | rogpeppe: returning nil when errPerm should be returned I think is wrong | 14:55 |
rogpeppe | dimitern: in this case, for this particular call, i think that's not true | 14:55 |
dimitern | rogpeppe: either that or because you're not authorized | 14:55 |
dimitern | rogpeppe: I don't see how this is different from Life for example | 14:56 |
rogpeppe | dimitern: sure. but i don't want to clutter the nice case for the sake of the rogue case (especially when it makes no difference either way) | 14:56 |
dimitern | rogpeppe: why should you be allowed to ask for life of arbitrary entities and not get errperm? | 14:56 |
dimitern | rogpeppe: security is important, even at expense of a bit more complicated ("cluttered") code | 14:57 |
rogpeppe | dimitern: because a life return actually tells you something about the entity | 14:57 |
dimitern | rogpeppe: even when it's an edge case - designing a good api should account for rouge agents | 14:57 |
rogpeppe | dimitern: saying "you cannot deploy this" does not tell you anything at all about the entity | 14:58 |
rogpeppe | dimitern: i agree entirely | 14:58 |
rogpeppe | dimitern: but there is no security implication from returning (false, nil) for not found | 14:58 |
rogpeppe | dimitern: it just makes the client code slightly nicer | 14:58 |
dimitern | rogpeppe: well, if we return false, nil in most cases, should we ever return errPerm then? | 14:58 |
rogpeppe | dimitern: i don't think so. | 14:59 |
dimitern | rogpeppe: so we're keeping the error return only in case there's a problem talking to state and nothing else? | 14:59 |
rogpeppe | dimitern: we have no control over the units that a machine agent asks about, but we can make sure it never finds out any info on units that it cannot deploy | 14:59 |
rogpeppe | dimitern: yes | 14:59 |
dimitern | rogpeppe: that's the reverse of what we agreed to do in general, but in this specific case i'm beginning to see your point | 15:00 |
dimitern | rogpeppe: at least I can't see any unnecessary leaks, it's just weird | 15:00 |
rogpeppe | dimitern: thanks. it is indeed a specific case. | 15:00 |
dimitern | rogpeppe: ok, will change it like that - no errPerm, only err if st.Unit() fails with !notFound | 15:01 |
rogpeppe | dimitern: something like this? http://paste.ubuntu.com/5904401/ | 15:02 |
dimitern | rogpeppe: not quite | 15:03 |
rogpeppe | dimitern: oops, http://paste.ubuntu.com/5904406/ | 15:04 |
dimitern | rogpeppe: AssignedMachineId returns NotAssignedErr most of the time and NotFound only in the case of a subordinate referring to an unknown principal | 15:04 |
rogpeppe | dimitern: ah, good point. | 15:04 |
rogpeppe | dimitern: perhaps just ignore the error from AssignedMachineId | 15:04 |
rogpeppe | dimitern: or http://paste.ubuntu.com/5904411/ | 15:05 |
rogpeppe | dimitern: (probably better) | 15:05 |
dimitern | rogpeppe: well I only really need is machineId != "" | 15:06 |
dimitern | rogpeppe: I don't think so | 15:07 |
dimitern | rogpeppe: if it indeed is notAssigned, then machineId == "" | 15:07 |
dimitern | rogpeppe: and that means false, nil basically | 15:07 |
dimitern | rogpeppe: (according to what we agreed, substituting errPerm for nil) | 15:07 |
rogpeppe | dimitern: yeah. depends if you care about returning some network error if you get one when callin AssignedMachineId | 15:08 |
dimitern | rogpeppe: will it be that bad to just ignore the err from assignedmachineid and return false, nil then? | 15:08 |
rogpeppe | dimitern: i'm thinking it would be a bit naughty but ok really | 15:09 |
rogpeppe | dimitern: actually, i think it is worth doing properly | 15:09 |
dimitern | rogpeppe: err != nil && machineId == "" -> false, nil | 15:10 |
rogpeppe | dimitern: otherwise a machine agent could potentially destroy a unit because of a db error | 15:10 |
dimitern | rogpeppe: possibly logging the err, what's wrong with that? | 15:10 |
dimitern | rogpeppe: no, if you're not responsible for it, you won't recall it | 15:10 |
dimitern | rogpeppe: or deploy it | 15:10 |
rogpeppe | dimitern: if the unit *does* happen to be assigned to the machine and there's a network error calling AssignedMachineId, you'll see (false, nil) and erroneously undeploy the unit | 15:11 |
dimitern | rogpeppe: it'll be recalled only if it's already deployed and you decide you're not responsible for it anymore | 15:12 |
rogpeppe | dimitern: thus i think it's worth checking for NotAssignedError explicitly | 15:12 |
dimitern | rogpeppe: yeah, good point | 15:12 |
rogpeppe | dimitern: yeah, but that's a problem if you're *told* you're not responsible for it, but you actually are | 15:12 |
dimitern | rogpeppe: so machineId == "" || err == NotAssigned -> false, nil then?, other err != nil -> false, err | 15:13 |
dimitern | rogpeppe: but that seems like a security leak, because we haven't yet checked the authOwner | 15:13 |
rogpeppe | dimitern: yes, in the hypothetical case that a malicious agent can induce a db error for the AssignedMachineId call but not the Unit call, we can leak information about the existence of a given unit | 15:16 |
rogpeppe | dimitern: i considered that, but really, i think it's vanishingly likely, and it's more important to behave well in the error case. | 15:17 |
rogpeppe | s/likely/unlikely :-) | 15:17 |
dimitern | rogpeppe: hmm | 15:19 |
dimitern | rogpeppe: it seems remote indeed | 15:19 |
dimitern | rogpeppe: the check has to be err == NotAssigned || err == NotFound || machineId == "" -> false, nil, else false, err | 15:20 |
rogpeppe | dimitern: not quite, but i'm sure you'll get it right in the code :-) | 15:20 |
dimitern | rogpeppe: why not? skip the notFound?\ | 15:21 |
rogpeppe | dimitern: it's more like: err != nil && err != NotAssigned && err != NotFound { return false, err} else if err != nil { return false, nil } else return AuthOwner(machineTag), nil | 15:22 |
dimitern | rogpeppe: it'll mean faulty state - a subordinate referring to an invalid principal, hence machineId of the principal cannot be determined | 15:22 |
dimitern | rogpeppe: ah, right :) | 15:22 |
rogpeppe | dimitern: but as i said, i'm sure you'll get it right | 15:22 |
dimitern | rogpeppe: will see :) | 15:23 |
rogpeppe | dimitern: thanks for bearing with me; sorry it's been a bit long-winded :-) | 15:23 |
dimitern | rogpeppe: np, I've seen some errors in my ways anyway :) | 15:23 |
dimitern | rogpeppe: and thanks | 15:23 |
rogpeppe | dimitern: i think it's useful to have this kind of conversation anyway around the security issues, so we know where we're coming from | 15:24 |
dimitern | rogpeppe: definitely | 15:24 |
TheMue | *pheeeeeeew* | 15:40 |
TheMue | still a bit ugly, but autosync works | 15:41 |
dimitern | TheMue: \o/ | 15:43 |
dimitern | rogpeppe: updated https://codereview.appspot.com/11713043/ | 15:52 |
rogpeppe | dimitern: reviewed | 15:57 |
dimitern | rogpeppe: thanks | 16:01 |
mramm | hey | 16:07 |
mramm | anybody know if the latest tools have been uploaded to the HP cloud public bucket? | 16:07 |
mramm | m_3 reports that he had to do upload tools with the latest, or else he got the previous version tools | 16:08 |
mgz | mramm: they may well not have been | 16:15 |
mgz | I'm not even certain who has the creds | 16:15 |
mramm | arosales: do you know? | 16:15 |
arosales | most folks here have creds. I can upload now though | 16:18 |
arosales | mgz, I think you have creds to the object store for juju-public-tools | 16:19 |
* arosales uploading now . . . | 16:19 | |
mgz | arosales: I see an email to jam were you added him, I guess I could have used those details... | 16:20 |
arosales | mgz, I think you have your own unique login | 16:22 |
mgz | arosales: if so, I can't find it in my email archive ;_; | 16:22 |
arosales | I think ian or someone asked for a set of folks to have access sometime in may | 16:22 |
arosales | mgz, no worries | 16:22 |
* arosales uploading now | 16:22 | |
mgz | do we also need to do anything simplestreams related? | 16:23 |
arosales | mgz, I think the public bucket has the image/tools info for AZ1 | 16:24 |
arosales | but AZ2/AZ3 aren't enabled, and haven't been :-/ | 16:24 |
arosales | mramm, 1.11.4 tools should be there now | 16:27 |
mramm | arosales: thanks! | 16:27 |
arosales | m_3, ^ | 16:27 |
rogpeppe | ha! reviewed everything! | 17:15 |
rogpeppe | some test race fixes, in case anyone fancies a review: https://codereview.appspot.com/11723043 | 17:39 |
rogpeppe | and... that's me for the day | 17:39 |
rogpeppe | see y'all tomorrow | 17:39 |
thumper | morning | 21:02 |
* thumper takes dog for a walk | 21:21 | |
thumper | trivial review for someone: https://codereview.appspot.com/11745043 | 23:00 |
thumper | hi mramm | 23:00 |
thumper | mramm: how goes oscon? | 23:00 |
mramm | great | 23:01 |
mramm | charm school this morning was good | 23:01 |
mramm | interest in juju from folks at intel, and several other places | 23:02 |
mramm | people seemed to get it | 23:02 |
thumper | cool | 23:02 |
mramm | gui and local provider were highlights | 23:04 |
mramm | lots of questions about LXC support in general | 23:04 |
mramm | and some definite interest in manual provisioning | 23:04 |
* thumper nods | 23:06 | |
thumper | it seems that mgz is on top of the network stuff, will try to touch base with him tonight my time, Wed morning his time | 23:06 |
mramm | cool | 23:06 |
thumper | I'm poking around some manual provisioning ideas | 23:06 |
mramm | In about an hour I have an hour of booth duty | 23:07 |
thumper | trying not to get too stressed about not knowing how the network stuff is going | 23:07 |
mramm | haha | 23:07 |
mramm | ;) | 23:07 |
thumper | juju booth or ubuntu booth? | 23:07 |
thumper | btw, what sort of questions around LXC? | 23:07 |
* davecheney waves | 23:58 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!