| === tasdomas is now known as tasdomas_afk | ||
| wallyworld_ | davecheney: normally, the name of a compiled Go executable takes after the package directory name. can i change this so that the binary is named something different? eg the directory is "metadata" but I want the binary to be called "juju-metadata" when i do a go install | 00:18 |
|---|---|---|
| thumper | wallyworld_: you may have to rename or symlink after the fact maybe? | 01:19 |
| wallyworld_ | thumper: yeah, i was trying to avoid that since most devs just do a "go install" and it will be wrong | 01:19 |
| * thumper nods | 01:20 | |
| thumper | package names can't have a - right? | 01:20 |
| wallyworld_ | right :-( | 01:20 |
| thumper | in which case you are rightly fcuked | 01:20 |
| wallyworld_ | we can add rules to the deb packaging config etc but i want devs to be able to use it easily also | 01:20 |
| wallyworld_ | i can also add a make target | 01:20 |
| wallyworld_ | but most core devs don't like the makefile :-( | 01:21 |
| wallyworld_ | it's all working apart from that | 01:21 |
| wallyworld_ | thumper: what i've done for now is make the package name "jujumetadata" so at least the binary has a reasonable, juju specific name in the default case where no rename is done as part of a local go install | 01:24 |
| thumper | wallyworld_: yes make a make-target | 01:25 |
| thumper | wallyworld_: and seems reasonable to have the long package name | 01:25 |
| * thumper looks for the default make target | 01:25 | |
| wallyworld_ | will do. so between the make target and the juju* package name we should be ok | 01:25 |
| thumper | make all | 01:26 |
| wallyworld_ | and i'll look into how our release scripts work | 01:26 |
| thumper | make install maybe | 01:26 |
| wallyworld_ | make install is good i think | 01:26 |
| * thumper nods | 01:26 | |
| thumper | there isn't one now | 01:26 |
| thumper | but should be easy enough | 01:26 |
| thumper | I will review that change if you like | 01:26 |
| thumper | I don't mind makefiles :) | 01:26 |
| axw | what doesn't allow "-" in the package name? | 01:26 |
| wallyworld_ | Go | 01:27 |
| axw | it does | 01:27 |
| axw | "." isn't | 01:27 |
| wallyworld_ | it complained when i tried | 01:27 |
| wallyworld_ | let me try again | 01:27 |
| axw | ah hang on | 01:27 |
| axw | sorry, I'm thinking of commands | 01:27 |
| axw | where the packagae name is "main" | 01:27 |
| wallyworld_ | axw: no, you are right. "-" is ok. not sure what i did the first time. thanks :-) | 01:29 |
| axw | wallyworld_: nps | 01:29 |
| wallyworld_ | thumper: it seems i need to provide a --description argument to the plugin. what is the intent there? isn't description similar to purpose? or? | 01:36 |
| thumper | wallyworld_: --description is shown when someone goes "juju help metadata" | 01:37 |
| thumper | no, that's help isn't it | 01:37 |
| wallyworld_ | yeah | 01:37 |
| thumper | --description is shown when someone goes "juju help plugins" | 01:37 |
| thumper | should be a one liner | 01:37 |
| wallyworld_ | yes | 01:37 |
| wallyworld_ | right, but purpose does that doesn't it? | 01:37 |
| thumper | what purpose? | 01:37 |
| wallyworld_ | ie cmd.Info.Purpose is the one liner | 01:37 |
| thumper | plugins are defined to be language agnostic | 01:38 |
| thumper | and entirely unrelated to our cmd stuff | 01:38 |
| wallyworld_ | ok. i'll just make the plugin print Purpose since it's in Go | 01:39 |
| jtv | Yay! The landing bot is running again then? | 02:20 |
| thumper | should be | 02:36 |
| jtv | Oh. But nothing's landed, and all my pending branches failed because the build environment wasn't fully set up. | 02:39 |
| jtv | Working now. Mine could be the first branches in! | 02:40 |
| thumper | jtv: you have a merge conflict | 02:49 |
| jtv | Inevitable. Thanks for the notice. | 02:49 |
| jtv | But another one got in. Two more to go. | 02:50 |
| jtv | thumper: oh good, it's all imports! | 02:51 |
| jtv | Wouldn't it be nice if we could automate that kind of resolution... | 02:51 |
| jtv | Throw all imports from the conflicting branches together, split them up into the 3 categories and format, try a build, remove any imports that the compiler says aren't needed. | 02:52 |
| wallyworld_ | thumper: i've found an issue with the plugin command. it was incorrectly dropping arguments meant for the plugin. there's a simple one line fix. i'll add a test also | 02:59 |
| thumper | wallyworld_: you need to pass -- | 02:59 |
| thumper | to stop the supercommand parsing the extra args | 02:59 |
| thumper | so | 02:59 |
| thumper | juju plug -for-juju -- -for-plugin | 02:59 |
| wallyworld_ | hmmm. ok. i did this change | 03:00 |
| wallyworld_ | --- cmd/juju/plugin.go 2013-07-30 01:31:52 +0000 | 03:00 |
| wallyworld_ | +++ cmd/juju/plugin.go 2013-07-30 02:57:22 +0000 | 03:00 |
| wallyworld_ | @@ -26,7 +26,7 @@ | 03:00 |
| wallyworld_ | flags := gnuflag.NewFlagSet(subcommand, gnuflag.ContinueOnError) | 03:00 |
| wallyworld_ | flags.SetOutput(ioutil.Discard) | 03:00 |
| meetingology | wallyworld_: Error: "@" is not a valid command. | 03:00 |
| wallyworld_ | plugin.SetFlags(flags) | 03:00 |
| wallyworld_ | - cmd.ParseArgs(plugin, flags, args) | 03:00 |
| wallyworld_ | + flags.Parse(false, args) | 03:00 |
| wallyworld_ | plugin.Init(flags.Args()) | 03:00 |
| thumper | schoool run | 03:00 |
| bigjools | afternoon | 03:33 |
| wallyworld_ | thumper: back? | 03:41 |
| thumper | yep | 03:41 |
| thumper | just had a coffee :) | 03:41 |
| thumper | nice | 03:41 |
| thumper | bigjools: how's tricks? | 03:42 |
| bigjools | thumper: awake, just, but twin1 is home at least | 03:42 |
| thumper | that must be a relief | 03:42 |
| wallyworld_ | thumper: with my change above, you no longer need the "--". without my change, the user can't run "juju metadata validate-image -r region" unless threy put in the "--" themselves which kinda sucks | 03:43 |
| wallyworld_ | with my change, the args just get flicked through to the plugin to handle | 03:44 |
| thumper | but plugins are environmental commands | 03:44 |
| thumper | so -e is valid | 03:44 |
| thumper | and -e needs to be handled by juju | 03:44 |
| thumper | to set the environment for the plugin | 03:45 |
| wallyworld_ | sure, -e works for me | 03:45 |
| wallyworld_ | as expected | 03:45 |
| thumper | -e does what? | 03:45 |
| wallyworld_ | pciks the env to use | 03:45 |
| wallyworld_ | juju metadata -e openstack validate-image -r foo | 03:45 |
| wallyworld_ | works | 03:45 |
| thumper | the tricky bit is where -e is parsed | 03:45 |
| thumper | is it parsed by juju | 03:45 |
| thumper | or the plugin? | 03:45 |
| wallyworld_ | hmmm. i think i made the plugin an env command | 03:46 |
| thumper | yeah... | 03:46 |
| wallyworld_ | so will need to check | 03:46 |
| thumper | so this would break other plugins | 03:46 |
| thumper | I do think the -- is the better solution | 03:47 |
| thumper | as sucky as it may be | 03:47 |
| wallyworld_ | i'll look into it a bit more. i really don't like the -- for built in plugins that are supposed to look like normal commands | 03:47 |
| thumper | yeah I know | 03:49 |
| thumper | but I'm not sure how to handle it better | 03:49 |
| thumper | without too much manual fuckery | 03:49 |
| thumper | will probably need work in gnuflag package | 03:50 |
| wallyworld_ | i'm hoping to avoid that | 03:50 |
| wallyworld_ | i'll see if i can insert the -- | 03:50 |
| wallyworld_ | since we are writing the "built in" plugin code | 03:50 |
| thumper | :) | 03:54 |
| === axw_ is now known as axw | ||
| thumper | axw: do you have a gravatar? | 05:11 |
| axw | thumper: umm yes | 05:12 |
| axw | should be on axwalk@gmail.com | 05:12 |
| axw | oh you mean for canonical.com? | 05:12 |
| * thumper marvals at the number of errors he just generated | 05:12 | |
| thumper | axw: if you attach your canonical email address to your gravatar, it shows up in leankit | 05:12 |
| axw | okey dokey | 05:13 |
| thumper | BOOM!!!! | 05:14 |
| thumper | that is what my tests say | 05:14 |
| * thumper should really focus on MAAS and not the logging stuff | 05:17 | |
| thumper | OMG we have some awful tests | 05:19 |
| bigjools | thumper: why are you surprised? | 05:20 |
| thumper | bigjools: I'm not really | 05:20 |
| bigjools | ah so that was more of an Oh Em Gee rather than a OMG | 05:21 |
| * thumper nods | 05:23 | |
| * thumper shelves this work | 05:23 | |
| thumper | it needs more focus | 05:23 |
| * thumper goes back to maas | 05:23 | |
| thumper | why does ssh use -p and scp use -P ? | 05:27 |
| * thumper pushes 24mb to bigjools | 05:27 | |
| bigjools | thumper: thanks a bunch :) | 05:28 |
| thumper | np | 05:28 |
| thumper | you have 35G free | 05:28 |
| thumper | sorry | 05:28 |
| thumper | 181G free | 05:28 |
| thumper | only 35 used | 05:28 |
| thumper | heaps of space | 05:28 |
| bigjools | yeah that machine can be happily destroyed | 05:29 |
| bigjools | I suspect my downlink is better than your uplink | 05:29 |
| thumper | bigjools: I bet it is too | 05:30 |
| thumper | 114.6KB/s to push over the ditch | 05:30 |
| * thumper bootstraps bigjools's maas | 05:33 | |
| thumper | I always get confused as whether to put the s in 's for something that ends with s | 05:34 |
| thumper | is there a good rule? | 05:34 |
| thumper | bigjools: how long until maas realises there is a bootstrap and updates the gui? | 05:34 |
| === tasdomas_afk is now known as tasdomas | ||
| bigjools | thumper: instant | 05:38 |
| bigjools | but it looks fooked | 05:38 |
| thumper | it didn't show anything | 05:38 |
| bigjools | yeah not here either | 05:38 |
| thumper | even though bootstrap supposedly succeeded | 05:38 |
| bigjools | refresh | 05:38 |
| thumper | ah, there we go | 05:38 |
| bigjools | could be a timeout on rabbit etcetc | 05:39 |
| thumper | kk | 05:39 |
| thumper | so now wait 20 minutes? | 05:39 |
| bigjools | yep | 05:39 |
| bigjools | the node powered itself up so should be ok | 05:40 |
| bigjools | you just deployed bare metal from 1500 miles away | 05:40 |
| thumper | \o/ | 05:41 |
| thumper | kinda cool | 05:41 |
| thumper | I'll come back later and poke the running instance | 05:41 |
| === thumper is now known as thumper-afk | ||
| rogpeppe1 | mornin' all | 06:38 |
| === TheMue_ is now known as TheMue | ||
| rogpeppe1 | i'd love a second review of this, please, if anyone wants to have a look. it's very small: https://codereview.appspot.com/12025043/ | 07:39 |
| rogpeppe1 | TheMue: ^ | 07:39 |
| TheMue | rogpeppe1: will look in a moment, my vm is just starting ;) | 07:41 |
| rogpeppe1 | TheRealMue: thanks | 07:42 |
| === TheRealMue is now known as TheMue | ||
| axw | so I had a branch that failed tests on the bot... due to some missing imports in files I didn't change | 07:44 |
| axw | I shouldn't need to manually merge from trunk first right? | 07:44 |
| axw | https://code.launchpad.net/~axwalk/juju-core/lp1182898-add-version-flag/+merge/176849 | 07:45 |
| axw | (now I have manually merged, and I need another review...) | 07:45 |
| TheMue | rogpeppe1: could you resend the link pls? | 07:45 |
| rogpeppe1 | TheMue: https://codereview.appspot.com/12025043/ | 07:45 |
| TheMue | rogpeppe1: thx | 07:45 |
| rogpeppe1 | TheMue: and https://codereview.appspot.com/12038045/ too, if you have a little more time | 07:46 |
| rogpeppe1 | TheMue: (i accidentally incorporated the first one into it, so please ignore the upgrader package changes in the second one - they'll disappear when the first one gets merged into trunk) | 07:47 |
| TheMue | rogpeppe1: first review done, looking at the second now ;) | 07:50 |
| rogpeppe1 | TheMue: thanks! | 07:51 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: TheMue | Bugs: 5 Critical, 80 High - https://bugs.launchpad.net/juju-core/ | ||
| axw | rogpeppe1: do you know what I need to do to make Go Bot happy here? | 07:54 |
| axw | https://code.launchpad.net/~axwalk/juju-core/lp1182898-add-version-flag/+merge/176849/comments/399702 | 07:54 |
| rogpeppe1 | axw: looking | 07:54 |
| rogpeppe1 | axw: oh yes, it's poxy | 07:54 |
| rogpeppe1 | axw: you need to reload the launchpad page before marking it Approved | 07:54 |
| rogpeppe1 | axw: i *think* that's probably the reason | 07:55 |
| axw | mmkay | 07:55 |
| rogpeppe1 | axw: it marks the mp as "approved at this particular revision" | 07:55 |
| axw | rogpeppe1: thanks, I'll give it a shot | 07:55 |
| axw | ah | 07:55 |
| rogpeppe1 | axw: it bites everyone | 07:56 |
| axw | do you know why I would've got those test failures in the previous comment? | 07:56 |
| axw | I didn't change any of that stuff in my branch | 07:57 |
| axw | trunk should always be clean... and there would've been no merge conflicts | 07:57 |
| TheMue | rogpeppe1: review is in | 08:00 |
| rogpeppe1 | TheMue: ta! | 08:00 |
| rogpeppe1 | axw: i'll have a look | 08:00 |
| rogpeppe1 | axw: i've no idea, sorry - i haven't seen anything like that before | 08:01 |
| rogpeppe1 | axw: i'd suspect mgz's bot setup | 08:01 |
| rogpeppe1 | axw: we'll see if my MP goes in ok | 08:01 |
| rogpeppe1 | axw: hmm, mine merged ok, so it looks like the bot *is* working | 08:02 |
| axw | rogpeppe1: nps, thanks for checking | 08:03 |
| rogpeppe1 | axw: see if it happens again | 08:03 |
| axw | rogpeppe1: I already merged manually and pushed, which is the wrong thing to do... I hope that doesn't screw anything up? | 08:03 |
| rogpeppe1 | axw: when the bot is working, that is frowned upon | 08:04 |
| rogpeppe1 | axw: but given that i'm sure you ran all the tests, it seems reasonable while the bot is flaky | 08:04 |
| axw | rogpeppe1: ok, sorry. hopefully it just works next time... | 08:05 |
| rogpeppe1 | axw: yeah | 08:05 |
| axw | rogpeppe1: when I say I merged manually, I mean I merged trunk into my branch and reapproved my MP | 08:06 |
| axw | I didn't push to the trunk directly | 08:06 |
| rogpeppe1 | axw: oh, that's standard practice | 08:06 |
| axw | rogpeppe1: phew :) | 08:06 |
| TheMue | :) | 08:13 |
| thumper | fwereade: ping | 08:24 |
| thumper | fwereade_: ping? | 08:25 |
| thumper | mgz: morning | 08:34 |
| mgz | hey thumper! | 08:35 |
| fwereade_ | thumper, pong | 08:38 |
| thumper | fwereade_: hey, was thinking we hadn't chatted in a while | 08:38 |
| thumper | fwereade_: and was wondering if you wanted to catch up | 08:38 |
| * thumper sits on the edge of his seat waiting for a response... | 08:41 | |
| thumper | mgz: did you want to catch up since I'm online anyway? | 08:46 |
| mgz | sure | 08:46 |
| thumper | mgz: skype, hangout, mumble? | 08:47 |
| mgz | anything but skype works for me :) | 08:48 |
| thumper | hangouts on my android phone really sucks the battery | 08:48 |
| thumper | mgz: so hangout is ok? | 08:48 |
| thumper | better than mumble normally | 08:48 |
| mgz | yup | 08:48 |
| mgz | axw, rogpeppe1: yeah, the bot doesn't want to resolve merge conflicts for you, it's not sentient | 08:57 |
| axw | mgz: fair enough ;) where can I see the merge conflicts? | 08:58 |
| axw | there weren't any when I did a manual merge | 08:58 |
| rogpeppe1 | mgz: ah, that error message didn't look like a merge conflict | 08:58 |
| mgz | axw: `bzr merge --preview -d trunk mybranch` or similar | 09:02 |
| axw | mgz: I already merged from trunk into my branch, and there were no conflicts | 09:04 |
| wallyworld | rogpeppe1: fwereade_: i've pushed changes to the validation branch. there's now built in plugin support, with the necessary changes to pass through args and help directives etc. so you can go 'juju help metadata' to get general plugin help listing the supported sub commands and 'juju help metadata validate-image' to get detailed help for that specifc command | 09:05 |
| axw | not sure why, but I got this failure: https://code.launchpad.net/~axwalk/juju-core/lp1182898-add-version-flag/+merge/176849/comments/399279 | 09:05 |
| axw | (I didn't touch any of those files) | 09:05 |
| fwereade_ | wallyworld, cool, tyvm | 09:05 |
| wallyworld | fwereade_: see what you think. but i'm quite happy with the ui and how it works | 09:06 |
| mgz | axw: er, sorry, confusing things, looking at the right branch now (it's actually merged now) | 09:06 |
| rogpeppe1 | wallyworld: great, thanks | 09:06 |
| wallyworld | btw, the binaries just get compiled and put in the normal bin directory | 09:07 |
| axw | mgz: I'm not sure if it was necessary, but I merged from trunk after I got that failure, and then pushed back to LP and reapproved the MP | 09:07 |
| mgz | axw: the message from the bot is complaining that you hit 'approve' in the merge proposal at r1541, then pushed (or launchpad didn't register the push until) the merge of trunk as r1542 | 09:08 |
| mgz | so, you had an extra rev that wasn't in the change that was 'approved' | 09:08 |
| axw | mgz: yep thanks, I understand that last error now | 09:08 |
| mgz | not really useful in this case as you're self landing | 09:08 |
| axw | just not sure why the original test failure occurred | 09:09 |
| axw | as there was never a merge conflict AFAICT | 09:09 |
| axw | and I didn't change those files | 09:09 |
| mgz | that mp also has a note from yesterday when the bot was all borked while I was fixing, that can be ignored | 09:09 |
| wallyworld | thumper: we probs need to talk to fwereade_ about how/if we should try and record what containers a particular instance supports | 09:09 |
| mgz | there was a general "don't approve things" in place, but you may not have realised as it was at the end of last week and I didn't restate yesterday | 09:10 |
| axw | mgz: okey dokey, sorry about that. I thought it was lifted | 09:10 |
| mgz | not until 6ish when I sent the email, but apart from confusing you did no harm :) | 09:11 |
| axw | ;) cool | 09:11 |
| rogpeppe1 | wallyworld: if i type "juju help commands", should i expect to see information on plugins there too? | 09:19 |
| wallyworld | rogpeppe1: no. you want to type juju help plugins | 09:21 |
| wallyworld | plugins can be bash scripts, python, whatever | 09:21 |
| rogpeppe1 | wallyworld: ah, that needs to be documented somewhere | 09:21 |
| wallyworld | rogpeppe1: it is isn't it? thumper did all that work | 09:22 |
| rogpeppe1 | wallyworld: oh yeah, it is | 09:22 |
| rogpeppe1 | wallyworld: hmm, i'm not sure that "juju help topics" should be separate from "juju help" but that's a matter for a different discussion | 09:23 |
| rogpeppe1 | wallyworld: BTW i'd expect "juju metadata generate-image --help" to work, but it doesn't | 09:24 |
| rogpeppe1 | wallyworld: what does it mean to be a "built-in plugin" | 09:26 |
| rogpeppe1 | ? | 09:26 |
| rogpeppe1 | wallyworld: i.e. why is the metadata plugin special like that? | 09:26 |
| rogpeppe1 | wallyworld__: what was the last thing you saw me say? | 09:27 |
| rogpeppe1 | wallyworld__: ping | 09:30 |
| wallyworld__ | rogpeppe1: hi, sorry was washing up after dinner. i saw you say "oh yeah" | 09:32 |
| rogpeppe1 | wallyworld__: ah, ok | 09:32 |
| rogpeppe1 | [10:23:09] <rogpeppe1> wallyworld: hmm, i'm not sure that "juju help topics" should be separate from "juju help" but that's a matter for a different discussion | 09:32 |
| thumper | rogpeppe1: juju help topics shows it | 09:33 |
| rogpeppe1 | 10:24:03] <rogpeppe1> wallyworld: BTW i'd expect "juju metadata generate-image --help" to work, but it doesn't | 09:33 |
| rogpeppe1 | [10:26:23] <rogpeppe1> wallyworld: what does it mean to be a "built-in plugin" | 09:33 |
| thumper | but the default help should have it shown too | 09:33 |
| rogpeppe1 | [10:26:24] <rogpeppe1> ? | 09:33 |
| rogpeppe1 | [10:26:49] <rogpeppe1> wallyworld: i.e. why is the metadata plugin special like that? | 09:33 |
| rogpeppe1 | thumper: i'd forgotten about help topics | 09:33 |
| rogpeppe1 | thumper: would it be unreasonable to merge "help topics" into "help" ? | 09:33 |
| thumper | rogpeppe1: that shows a bit much | 09:33 |
| thumper | especially as I'm trying to get more topics | 09:34 |
| thumper | like constraints | 09:34 |
| wallyworld__ | rogpeppe1: the --help should work i think. i'd been testing with help. plugin arg handling is a little different | 09:34 |
| thumper | and other... | 09:34 |
| wallyworld__ | rogpeppe1: built in just means it is shipped with juju | 09:34 |
| rogpeppe1 | thumper: i wouldn't mind seeing a few more lines in juju help | 09:34 |
| wallyworld__ | out of the box | 09:34 |
| rogpeppe1 | wallyworld__: why does "built-in" imply "doesn't allow interspersed flags" ? | 09:35 |
| thumper | rogpeppe1: I'd ask jcastro and marco what they think | 09:35 |
| rogpeppe1 | thumper: +1 | 09:35 |
| rogpeppe1 | wallyworld__: also, isn't the very point of a plugin that it's external to the main executable? | 09:36 |
| wallyworld__ | rogpeppe1: because that's what is needed to make the flag handling pass all the args to the next command invoked | 09:36 |
| wallyworld__ | rogpeppe1: it is external | 09:36 |
| wallyworld__ | it's a separate binary | 09:36 |
| rogpeppe1 | wallyworld__: so why are built-in plugins special in their flag-handling requirements? | 09:37 |
| wallyworld__ | rogpeppe1: all plugins are | 09:37 |
| wallyworld__ | but external plugins require -- | 09:38 |
| wallyworld__ | and i'd rather make the built in ones look "native" | 09:38 |
| wallyworld__ | and not make the user type "--" | 09:38 |
| rogpeppe1 | wallyworld__: that seems like unreasonable special-casing to me | 09:39 |
| rogpeppe1 | wallyworld__: a plugin should be a plugin | 09:39 |
| rogpeppe1 | wallyworld__: line 30 of https://codereview.appspot.com/11566043/diff/54001/cmd/juju/plugin.go seems weird to me | 09:39 |
| wallyworld__ | i disagree in this case. we don't want the user having to know to type "--" just to use functionality shipped with thr system | 09:40 |
| wallyworld__ | i made it a plugin to get it out of the main juju help topic | 09:40 |
| rogpeppe1 | wallyworld__: if we want to make it a plugin, i think it should be a normal plugin. | 09:40 |
| wallyworld__ | i disagree | 09:40 |
| rogpeppe1 | wallyworld__: if normal plugins are hard to use, that's another issue | 09:40 |
| wallyworld__ | it needs to be integrated better than that | 09:40 |
| rogpeppe1 | wallyworld__: if we want it to be integrated, it should not be a plugin | 09:41 |
| rogpeppe1 | wallyworld__: this seems to me to be the worst of both worlds | 09:41 |
| wallyworld__ | well, it was integrated | 09:41 |
| rogpeppe1 | wallyworld__: i know, and we decided to make it a plugin | 09:41 |
| wallyworld__ | but then you guys wanted it a little more separated | 09:41 |
| rogpeppe1 | wallyworld__: so i think it should be just that. no special casing. | 09:41 |
| wallyworld__ | yes, to uncluter juju help commands | 09:41 |
| wallyworld__ | but i do not want to make it hard for the user | 09:41 |
| rogpeppe1 | wallyworld__: this just makes a *third* category of juju subcommand | 09:42 |
| wallyworld__ | users are not unix geeks | 09:42 |
| rogpeppe1 | wallyworld__: which actually makes it harder for the user, IMO | 09:42 |
| rogpeppe1 | wallyworld__: because they don't know what's going to happen (for example) to the -e flag | 09:42 |
| wallyworld__ | they do | 09:42 |
| wallyworld__ | -e behaves as expected | 09:42 |
| wallyworld__ | it selects an env to use | 09:42 |
| rogpeppe1 | wallyworld__: it doesn't for metadata generate-image | 09:42 |
| wallyworld__ | i'd have to check that command, i can't recall if it needs an env or not | 09:43 |
| rogpeppe1 | wallyworld__: it takes a -e flag that is *not* an environment | 09:43 |
| wallyworld__ | so that should change perhaps. should -e e reserved across all commands to only mean env? | 09:44 |
| wallyworld__ | be | 09:44 |
| rogpeppe1 | wallyworld__: it is reserved in that way for all normal commands (and normal plugins too, i think) | 09:44 |
| rogpeppe1 | wallyworld__: except when -- is used | 09:44 |
| wallyworld__ | ok. i'll change that arg name then | 09:45 |
| wallyworld__ | rogpeppe1: actually, that was supposed to be -u for that one | 09:46 |
| rogpeppe1 | wallyworld__: i'd be much happier if all the "built-in plugin" stuff was removed too. it's almost self contradictory, and very odd, i think. | 09:46 |
| wallyworld__ | why? i disagree | 09:46 |
| rogpeppe1 | wallyworld__: a plug-in is by definition not built-in, i think. | 09:46 |
| wallyworld__ | what? photoshop and other things ship with built in plug ins | 09:47 |
| wallyworld__ | lots of software does | 09:47 |
| wallyworld__ | the plugin mechanism is a framework | 09:47 |
| wallyworld__ | software can ship with built in tools that use the framework. and allow for external extension | 09:47 |
| rogpeppe1 | wallyworld__: it seems like we're giving this particular plug-in special privileges because it's awkward to use as a normal plug-in. don't you think that other people providing plug-ins might have similar requirements? but they won't be able to switch on the "built-in" flag. | 09:48 |
| rogpeppe1 | wallyworld__: perhaps we should make all plugins parse all their own flags, including -e | 09:49 |
| wallyworld__ | with externally written plugins, my mental model is that they are bolted on type thing. hence one has slightly different expectations | 09:50 |
| rogpeppe1 | wallyworld__: how can the user possibly know which things are externally written and which are not? | 09:50 |
| wallyworld__ | the plugins do parse their own flags i think, even the built in ones | 09:50 |
| rogpeppe1 | wallyworld__: i don't think they should have to know | 09:50 |
| rogpeppe1 | wallyworld__: i think the standard flags are parsed before passing to the plugin | 09:51 |
| rogpeppe1 | wallyworld__: and that's why allowIntersperse is true | 09:51 |
| rogpeppe1 | wallyworld__: i agree that it's not a great user experience, because they need to know that -e is parsed by juju proper, and the other flags are parsed by the plugin itself | 09:52 |
| wallyworld__ | so the user types "juju help metadata validate-image" and they get told hoe to use the command. they don't really need to know it's a plugin | 09:52 |
| rogpeppe1 | wallyworld__: and hence require a "--" separator | 09:52 |
| rogpeppe1 | wallyworld__: the juju only *knows* about the metadata command because they typed "juju help plugins" | 09:52 |
| wallyworld__ | yes, that's how you wanted it! | 09:53 |
| wallyworld__ | otherwise i wouldn't have bothered with the work to make it a plugin | 09:53 |
| rogpeppe1 | wallyworld__: i defer to fwereade_ | 09:53 |
| wallyworld__ | sorry. discussions on irc suck | 09:54 |
| rogpeppe1 | wallyworld__: personally, i was ok with it being either a fully fledged plugin, or a fully builtin (set of) command(s). | 09:54 |
| rogpeppe1 | wallyworld__: but this half-way house seems bad to me | 09:54 |
| rogpeppe1 | wallyworld__: it's like "it's a plugin but it's not really" | 09:54 |
| wallyworld__ | the only difference i think is the use of "--" or not | 09:55 |
| rogpeppe1 | wallyworld__: yes. and without that difference, would you actually *need* the concept of built-in plugins? | 09:55 |
| fwereade_ | wallyworld__, I suspect rogpeppe1 has a point here, but I'll have to look closer | 09:55 |
| wallyworld__ | as a user, i would HATE to type "--" just to validate some image metdata | 09:56 |
| rogpeppe1 | wallyworld__: then as a user you'll hate to use any juju plugin | 09:56 |
| rogpeppe1 | wallyworld__: which means that our plugin concept is broken | 09:56 |
| wallyworld__ | maybe. but external ones give different expectations | 09:56 |
| wallyworld__ | built in ones need to be more integrated | 09:57 |
| rogpeppe1 | wallyworld__: so let's fix that rather than providing a special-case hack because we don't like the way they work | 09:57 |
| wallyworld__ | since they ship with the system | 09:57 |
| rogpeppe1 | wallyworld__: i think all plugins should work well | 09:57 |
| axw | good night all | 09:57 |
| rogpeppe1 | axw: cheerio | 09:57 |
| rogpeppe1 | wallyworld__: and if we can't make the ones that ship with the system work well without special-case hacks, then we're doing something wrong | 09:57 |
| wallyworld__ | why did we force people to use "--" in the first place? was that a limitation of the gnu flags package? | 09:58 |
| rogpeppe1 | wallyworld__: i agree with your motivation, BTW. i think it's nasty that the user has to add a "--" to pass flags to a plugin | 09:58 |
| rogpeppe1 | wallyworld__: in a way, yes | 09:59 |
| rogpeppe1 | wallyworld__: but actually it's a fundamental limitation of the gnu flag syntax | 09:59 |
| rogpeppe1 | wallyworld__: you can't parse flags without knowing what flags are expected | 09:59 |
| rogpeppe1 | wallyworld__: so what we'd like to do is parse *only* the standard flags and leave the rest for parsing by the plugin | 10:00 |
| wallyworld__ | sounds reasonable | 10:00 |
| rogpeppe1 | wallyworld__: but that's impossible in general | 10:00 |
| wallyworld__ | so if we are going to fix the "--" i'd really like to land this as is and follow up separately | 10:00 |
| wallyworld__ | we know if we are processing a "missing command". can't we be less strict on the flags processing in that case | 10:01 |
| rogpeppe1 | wallyworld__: i'd much prefer this to land with the command as a normal plugin, without the special-case built-in stuff. it makes the CL more huge than it needs to be | 10:01 |
| wallyworld__ | and just pass what we don;t know about through | 10:01 |
| rogpeppe1 | wallyworld__: that's what i was saying - that's actually impossible | 10:02 |
| wallyworld__ | i don't want to impose the "--" on our users | 10:02 |
| wallyworld__ | built in means we get to do a better job of integrating | 10:02 |
| rogpeppe1 | wallyworld__: if "better job of integrating" means "we don't need to pass --", and we're planning to fix that, then i don't see the point | 10:03 |
| wallyworld__ | like car accessories or whatever. stuff that comes with the vehicle out of the factory is alsways better than going to the auto shop and bolting on later | 10:03 |
| rogpeppe1 | wallyworld__: let's not design it like that, please | 10:03 |
| wallyworld__ | this is about getting much needed functionality into user's hands with the desired interface | 10:04 |
| wallyworld__ | the built in handling is behind the scenes and can be fixed later for any existing plugins | 10:04 |
| wallyworld__ | to remove the "--" | 10:04 |
| rogpeppe1 | wallyworld__: the functionality is much more important than the interface at this point | 10:04 |
| wallyworld__ | the ui is important too | 10:05 |
| rogpeppe1 | wallyworld__: and this is unnecessary cruft in our code base | 10:05 |
| wallyworld__ | i don't want to ship something only to change the interface latert | 10:05 |
| wallyworld__ | its a few lines to handle tie "--" | 10:05 |
| wallyworld__ | the bulk of the code is business logic | 10:05 |
| wallyworld__ | we need to deliver incrementally | 10:06 |
| rogpeppe1 | wallyworld__: speaking of delivering incrementally, that CL should really be split into about 3 or 4. | 10:06 |
| rogpeppe1 | wallyworld__: i appreciate that you want to deliver the result though | 10:06 |
| wallyworld__ | it was smallish but then it grew | 10:07 |
| rogpeppe1 | wallyworld__: but i'm sure you can deliver it and say "yes, the interface is currently provisional, but we're working on it" | 10:07 |
| rogpeppe1 | wallyworld__: yeah, i know the feeling very well | 10:07 |
| wallyworld__ | well, i'd like the interface not to be provisonal | 10:07 |
| wallyworld__ | i like not having the "--" | 10:07 |
| rogpeppe1 | i'd very much like not to put "temporary" hacks into the code base which may need to stay forever | 10:08 |
| wallyworld__ | i'd like to get this out there and then fix how existing external plugins work | 10:08 |
| wallyworld__ | sometimes we need to be pragmatic to be agile | 10:08 |
| rogpeppe1 | wallyworld__: how about fixing things by always passing "false" to flags.Parse ? | 10:09 |
| wallyworld__ | that will break external plugins | 10:09 |
| rogpeppe1 | wallyworld__: i think that might actually be a nicer solution actually | 10:09 |
| rogpeppe1 | wallyworld__: i don't think so | 10:09 |
| rogpeppe1 | wallyworld__: i don't think it will affect them at all | 10:09 |
| wallyworld__ | hmmm. i thought i tested it and it broke. tim seemed to think it would break stuff also. i can look again. i originally had it that way and tim had concerns | 10:10 |
| rogpeppe1 | wallyworld__: it just means that you won't be able to do "juju some-plugin -e my-env" unless the plugin actually implements the -e flag | 10:10 |
| wallyworld__ | perhaps what you say above was the issue. can't recall now | 10:11 |
| rogpeppe1 | wallyworld__: hmm, i'm not sure if "juju -e my-env some-plugin" will work actually | 10:11 |
| wallyworld__ | i'm not sure it would be wise to possibly break existing plugins | 10:11 |
| rogpeppe1 | wallyworld__: *are* there any existing plugins? | 10:11 |
| wallyworld__ | yes, because the guys in oakland were *really* keen for them, that's why plugins were written | 10:12 |
| wallyworld__ | mainly mark mimms and his group | 10:12 |
| rogpeppe1 | wallyworld__: i know they were keen for them, but have they actually written any yet? | 10:12 |
| wallyworld__ | i *think* so. i thought they wrote some while we were there | 10:13 |
| wallyworld__ | or re-purposed existing code as a plugin | 10:13 |
| wallyworld__ | rogpeppe1: let's finalise what we want to do with fwereade_after the standup maybe | 10:14 |
| rogpeppe1 | wallyworld__: sgtm | 10:14 |
| wallyworld__ | much easier than typing | 10:15 |
| * wallyworld__ goes to open some more wine to prepare :-) | 10:15 | |
| rogpeppe1 | wallyworld__: sorry for the bother | 10:16 |
| wallyworld__ | rogpeppe1: no problem. it's a necessary discussion. i'd love to get rid of the "--" | 10:17 |
| wallyworld__ | irc is so impersonal | 10:18 |
| rogpeppe1 | wallyworld__: BTW the fundamental difficulty with extracting the -e flags without disrupting the rest of the flags is that it can break command lines like: juju my-plugin --title $title | 10:19 |
| rogpeppe1 | wallyworld__: consider what happens if $title happens to be '-environment-' | 10:20 |
| wallyworld__ | wouldn't the flags know that -title requires a value | 10:20 |
| wallyworld__ | and hence -env- is the value | 10:21 |
| wallyworld__ | that goes with -title | 10:21 |
| rogpeppe1 | wallyworld__: no, because we're parsing them before my-plugin gets called | 10:21 |
| wallyworld__ | so maybe the flags that are typed after plugin are considered to belong to plugin are ignored by juju | 10:21 |
| rogpeppe1 | wallyworld__: that's the main (only?) advantage that plugins provide for you, other than that you can call them with "juju my-plugin" | 10:21 |
| rogpeppe1 | wallyworld__: i think that's a reasonable p.o.v. | 10:22 |
| rogpeppe1 | wallyworld__: but it means that there's not necessarily any standardisation on -e/--environment | 10:22 |
| rogpeppe1 | wallyworld__: it's what i was suggesting by passing allowIntersperse=false | 10:22 |
| wallyworld__ | so 'juju -e env -log debug my-plugin -t $title' | 10:23 |
| rogpeppe1 | wallyworld__: yeah, i think allowing a -e flag to *any* juju command isn't a bad idea | 10:23 |
| wallyworld__ | well, -e would be for juju only i guess | 10:23 |
| rogpeppe1 | wallyworld__: currently plugins will work when called as "juju some-plugin -e my-env", same as other commands (and also currently, a plugin doesn't have to interpret the -e flag). | 10:27 |
| rogpeppe1 | wallyworld__: this would break that | 10:28 |
| wallyworld__ | hmmm. maybe we can live with that. not sure | 10:28 |
| rogpeppe1 | wallyworld__: i'm not sure that's that important. i think it actually simplies juju itself, at the expense of making plugins marginally harder to implement (you need to parse the -e flag) | 10:29 |
| rogpeppe1 | (rather than read the JUJU_ENV environment variable) | 10:29 |
| wallyworld__ | yeah. worth considering | 10:30 |
| rogpeppe1 | quick shell script question: easiest way to sort a Go traceback by goroutine id? | 10:47 |
| mgz | rogpeppe1: I thought you were the one with all the fancy awk stuff for understanding go panics... | 10:48 |
| rogpeppe1 | mgz: not afair... | 10:49 |
| rogpeppe1 | mgz: though for tracebacks with hundreds of goroutines, i've been wondering about something to make them a bit more accessible | 10:49 |
| mgz | indeed. | 10:50 |
| rogpeppe1 | mgz: something that would classify similar ones together, for example | 10:50 |
| rogpeppe1 | i thought that someone might have a two line piece of python magic :-) | 10:51 |
| rogpeppe1 | for the record: http://paste.ubuntu.com/5928595/ | 11:04 |
| TheMue | aaaaah, got it, tests are passing again *phew* | 11:08 |
| dimitern | fwereade: https://codereview.appspot.com/12034043/ | 11:24 |
| fwereade_ | dimitern, sorry, I haven't done that very well, I just had a couple of comments I forgot to send | 11:30 |
| fwereade_ | dimitern, I think rogpeppe1's comments generally made sense | 11:30 |
| dimitern | fwereade: ok | 11:31 |
| mgz | standup all | 11:31 |
| dimitern | fwereade: will repropose with his suggestions after the standup | 11:31 |
| fwereade_ | dimitern, cool, cheers | 11:32 |
| benji | gary_poster: nope; when I changed the source yesterday the web socket never would reconnect; I think I didn't do the self-signed cert acceptance dance right. | 12:24 |
| benji | gggfddsfds; wrong chan | 12:24 |
| rogpeppe1 | fwereade_: when you have a moment, i wouldn't mind having that chat about EnsureAPIInfo | 12:35 |
| fwereade_ | rogpeppe1, one-on-one atm, then lunch I fear | 12:36 |
| rogpeppe1 | fwereade_: np | 12:36 |
| * rogpeppe1 should have some lunch too | 12:36 | |
| rogpeppe1 | fwereade_: i'm tempted to drop EnsureAPIInfo entirely | 12:37 |
| rogpeppe1 | fwereade_: anyone that needs to upgrade from 1.10 can upgrade to 1.12 first | 12:37 |
| jcastro | hey guys | 13:37 |
| jcastro | distro go this MP | 13:37 |
| jcastro | https://code.launchpad.net/~tribaal/ubuntu/saucy/juju-core/add-bash-completion/+merge/177287 | 13:38 |
| jcastro | but they feel this should be upstream | 13:38 |
| jcastro | anyone know how I can put this on your merge radar instead of distros? | 13:38 |
| rogpeppe1 | jcastro: interesting. i can't answer your question, but it makes me more convinced that "juju help commands" should print all commands, including plugins | 13:46 |
| jcastro | so ... assign thumper | 13:46 |
| jcastro | Is that what you meant? :) | 13:47 |
| ahasenack | hm, I have my unit logs over on the unit full of these: | 13:49 |
| ahasenack | error: --unit-name option expects "<service>/<n>" argument | 13:49 |
| ahasenack | that's all it has | 13:49 |
| ahasenack | any idea what command it was trying to run? | 13:49 |
| ahasenack | aha | 13:50 |
| ahasenack | exec /var/lib/juju/tools/unit-rabbitmq-server-0/jujud unit --data-dir /var/lib/juju --unit-name rabbitmq/server/0 --debug >> /var/log/juju/unit-rabbitmq-server-0.log 2>&1 | 13:50 |
| rogpeppe1 | jcastro: quite possibly :-) | 13:53 |
| rogpeppe1 | ahasenack: can you reproduce the problem from scratch? | 13:53 |
| ahasenack | rogpeppe1: I'm not sure, investigating still | 13:54 |
| ahasenack | I don't see any obvious typos so far | 13:54 |
| jtv | Any reviewers available for https://codereview.appspot.com/12103043 ? | 13:56 |
| dimitern | jcastro: yeah, there are plugins as well | 13:56 |
| ahasenack | guys, what do you think about an option to destroy-unit like this: | 14:02 |
| ahasenack | juju destroy-unit foo/0 --terminate | 14:02 |
| ahasenack | it would also terminate the machine | 14:03 |
| ahasenack | or even for destroy-service too | 14:03 |
| dimitern | ahasenack: you can try destroy-machine directly | 14:03 |
| ahasenack | oh? | 14:03 |
| * ahasenack checks that one out | 14:03 | |
| ahasenack | dimitern: doesn't work | 14:04 |
| ahasenack | Machines that have assigned units, or are responsible for the environment, cannot be destroyed. | 14:04 |
| ahasenack | dimitern: right now the sequence is | 14:04 |
| ahasenack | dimitern: juju status, lookup machine id of the unit you are about to destroy | 14:04 |
| ahasenack | dimitern: juju destroy-unit unit/0 | 14:04 |
| ahasenack | dimitern: wait a few seconds | 14:04 |
| ahasenack | dimitern: juju terminate-machine <machiine-id-looked-up-earlier> | 14:04 |
| dimitern | ahasenack: if you do terminate-machine you don't need to do destroy-unit before that i think | 14:05 |
| ahasenack | dimitern: the help says it won't destroy the machine | 14:05 |
| ahasenack | dimitern: and it's an alias to terminate-machine, which I can confirm doesn't do that. It stops if the machine still has a unit | 14:05 |
| ahasenack | I mean, destroy-machine (what you said before) is an alias to terminate-machine, which is what I have been using | 14:06 |
| dimitern | ahasenack: ah, you're right yes | 14:06 |
| rogpeppe1 | ahasenack: i think it should be possible to terminate any machine with destroy-machine, and it should do what's necessary to bring that about, but others (notably fwereade_) may have different opinions | 14:10 |
| rogpeppe1 | fwereade_: ping | 14:10 |
| ahasenack | I would be happy with a --force option to destroy-machine, or --terminate option for destroy-unit | 14:10 |
| ahasenack | more the latter | 14:11 |
| ahasenack | just reading the --force docs would make me wonder if destroy-machine --force would also call destroy-unit, i.e., everything should happen in the right order | 14:11 |
| mgz | the second doesn't really make sense in the multipl-units-per-machine case | 14:11 |
| ahasenack | destroy-unit --terminate would mean 1) destroy-unit, run all the hooks that are needed; 2) terminate-machine | 14:11 |
| ahasenack | ah, there is that now | 14:12 |
| ahasenack | it could be so that --terminate would only terminate if the machine is vacant | 14:12 |
| ahasenack | because this workflow I described is something I use a *lot* | 14:12 |
| ahasenack | I destroy a unit or service, in preparation for a redeploy/add-unit, and I need the machine terminated, or else it will be used again as it was left | 14:13 |
| mgz | well, and with containerisation, machine reuse will be... less undesirable | 14:13 |
| ahasenack | what I'm doing now basically is looping over all machine ids and calling terminate, and letting juju decide if it can terminate the machine or not | 14:13 |
| mgz | because destrying the unit can just blow away the container | 14:13 |
| dpb1 | ahasenack: I don't think machines are "recyled" anymore | 14:13 |
| ahasenack | dpb1: still, the dangling machine builds up against my quota | 14:14 |
| dpb1 | ahasenack: yes, that is true | 14:14 |
| rogpeppe1 | mgz: wondering about your thoughts on this: i'm considering making a change that will mean we can't upgrade a juju environment from 1.10 to current, although you'll be able to upgrade via 1.12. do you think that's reasonable? | 14:14 |
| rogpeppe1 | mgz: basically, i'm trying to avoid one temporary hack spreading tentacles | 14:15 |
| rogpeppe1 | fwereade_: if you're around, i'm talking about the EnsureAPIInfo hack here | 14:15 |
| mgz | personally, I don't care about upgrades at all, so maybe poke william :) | 14:16 |
| mgz | I always blow all my stuff away and redeploy | 14:16 |
| === BradCrittenden is now known as bac | ||
| rogpeppe1 | mgz: yeah, me too. but you might know of people that have stayed on 1.10 for whatever reason, peraps | 14:17 |
| mgz | having random point in trunk borked is always fine I think, but by "current" do you also mean rolling forward to 1.14? | 14:17 |
| rogpeppe1 | mgz: yeah | 14:17 |
| rogpeppe1 | mgz: if you're running 1.10, you'd need to upgrade via 1.12 | 14:17 |
| mgz | that might be more of an issue, because we do probably need some kind of upgrade path for what we've shipped in raring to what we'll ship in saucy | 14:17 |
| rogpeppe1 | mgz: well, there *will* be an upgrade path | 14:18 |
| rogpeppe1 | mgz: just that it's via 1.12 | 14:18 |
| rogpeppe1 | mgz: but i guess if people can't readily get access to that, then, yeah, it might be a problem | 14:18 |
| rogpeppe1 | mgz: darnit | 14:18 |
| sidnei | hey folks, can i get some attention to https://bugs.launchpad.net/juju-core/+bug/1205112 | 14:18 |
| _mup_ | Bug #1205112: panic while setting a config value <juju-core:Triaged> <https://launchpad.net/bugs/1205112> | 14:18 |
| rogpeppe1 | sidnei: i'd like to help, but i didn't see a description of how to reproduce the problem | 14:19 |
| rogpeppe1 | sidnei: i.e. have you got a way that i can reproduce it from scratch? | 14:20 |
| rogpeppe1 | sidnei: or, alternatively, can you reproduce it reliably on tip? | 14:20 |
| rogpeppe1 | sidnei: (in which case, i'd ask you to add a debug print in a particular place, which should help diagnose the problem) | 14:21 |
| sidnei | rogpeppe1: i can look into that. im suspecting it may be charm specific even. i think it's reliably reproducible. | 14:21 |
| rogpeppe1 | sidnei: i took a look at the code and couldn't see how the crash is possible | 14:21 |
| rogpeppe1 | sidnei: thanks | 14:21 |
| sidnei | rogpeppe1: i'm doing juju deploy --config, and only about half settings got set, and then trying to set any setting after that causes the panic. i think there might be some error during the deploy --config which is in the logs but i haven't looked there. | 14:22 |
| rogpeppe1 | sidnei: that's entirely possible | 14:22 |
| rogpeppe1 | sidnei: can you paste the config file, please? | 14:22 |
| rogpeppe1 | sidnei: i suspect that's the source of the problem | 14:23 |
| sidnei | rogpeppe1: http://paste.ubuntu.com/5929199/ | 14:23 |
| sidnei | rogpeppe1: http://paste.ubuntu.com/5929208/ is the charms' config.yaml | 14:24 |
| rogpeppe1 | sidnei: u1-stream is the name of the service, presumably? | 14:26 |
| sidnei | rogpeppe1: correct | 14:26 |
| rogpeppe1 | sidnei: is it significant that it's different from the service name in the bug report, or is that just a name change since then? | 14:27 |
| sidnei | rogpeppe1: it's the same charm, different service name yes. | 14:27 |
| sidnei | rogpeppe1: worth noting that the panic happens setting any config value, not just a specific one. my hunch is that there's something persisted from the --config that causes the comparison to blow up. | 14:28 |
| rogpeppe1 | sidnei: are you seeing this against juju tip? | 14:28 |
| rogpeppe1 | sidnei: (yes, that sounds very plausible) | 14:29 |
| sidnei | rogpeppe1: tip-ish, revno 1544, i can update and try again. | 14:29 |
| rogpeppe1 | sidnei: no, it's ok. i'll just give you a debug statement to add, and perhaps you could try again? | 14:29 |
| fwereade_ | rogpeppe1, sidnei: gut suspicion: the dotted config names are treated specially by mongo, because santitizing our inputs? what's that? | 14:29 |
| rogpeppe1 | fwereade_: i think you might have a good point there | 14:30 |
| sidnei | fwereade_: likely. | 14:30 |
| fwereade_ | rogpeppe1, sidnei: and they all get packed into $update foo.bar -> dict -> blam | 14:30 |
| rogpeppe1 | fwereade_: we just use the names directly | 14:30 |
| sidnei | fwereade_: i've used dotted config names since pyjuju supported them, to map into the charm's service dotted names directly. | 14:31 |
| rogpeppe1 | fwereade_: yeah, and since there are several names for, say keystone, they'll be unmarshalled into a map perhaps | 14:31 |
| fwereade_ | rogpeppe1, man, that Settings type pisses me off in more ways than I can count at this point | 14:31 |
| rogpeppe1 | fwereade_: :-) | 14:31 |
| rogpeppe1 | fwereade_: this is another reason for having some charm name restrictions | 14:31 |
| rogpeppe1 | fwereade_: so at least we know the alphabet we're dealing with | 14:32 |
| fwereade_ | rogpeppe1, +100 | 14:32 |
| rogpeppe1 | fwereade_: i suggest that for the time being, we can just sanitise settings names | 14:32 |
| rogpeppe1 | fwereade_: map . to... something | 14:32 |
| fwereade_ | rogpeppe1, yeah, mumble grumble compatibility grumble, but yeah | 14:33 |
| rogpeppe1 | fwereade_: we'll need to map back again too, of course | 14:33 |
| * rogpeppe1 goes to read about mongo name syntax | 14:33 | |
| fwereade_ | rogpeppe1, tbh all those keys should go in a subdict of the doc *anyway*, because one day someone is going to be baffled by the effect of trying to call a key txn-revno, or similar | 14:35 |
| fwereade_ | rogpeppe1, but yes also sanitised | 14:36 |
| rogpeppe1 | fwereade_: agreed | 14:36 |
| rogpeppe1 | fwereade_: looks like we need to sanitize (at least) : "$", "." and leading digits | 14:38 |
| dimitern | rogpeppe1, fwereade: updated https://codereview.appspot.com/12034043 | 14:39 |
| fwereade_ | rogpeppe1, ok, and now I come to think of it compatibility isn't an issue | 14:39 |
| fwereade_ | rogpeppe1, none of those will work anyway ;p | 14:40 |
| rogpeppe1 | fwereade_: indeed | 14:40 |
| rogpeppe1 | fwereade_: although we need to think carefully about our sanitisation | 14:40 |
| dimitern | please take a look, i need to land this already - it's too huge to stay long, otherwise i'll run into some nasty conflicts | 14:40 |
| fwereade_ | dimitern, will do | 14:41 |
| dimitern | cheers! | 14:41 |
| * dimitern bbiab | 14:51 | |
| rogpeppe1 | dimitern: reviewed | 15:10 |
| rogpeppe1 | fwereade_: chat about EnsureAPIInfo? | 15:10 |
| fwereade_ | rogpeppe1, sure, couple of mins, would you start one please? | 15:12 |
| rogpeppe1 | fwereade_: will do | 15:12 |
| rogpeppe1 | fwereade_: https://plus.google.com/hangouts/_/f497381ca4d154890227b3b35a85a985b894b471?authuser=1 | 15:12 |
| dimitern | rogpeppe1: thanks! | 15:20 |
| fwereade_ | dimitern, reviewed | 15:23 |
| dimitern | fwereade: cheers | 15:25 |
| jtv | Any reviewers in the house? Looking for someone to take a look at https://codereview.appspot.com/12103043 | 15:52 |
| rogpeppe1 | jtv: swap ya: https://codereview.appspot.com/12114043 | 16:03 |
| rogpeppe1 | anyone else too - it's a trivial deletion-only CL | 16:03 |
| dimitern | rogpeppe1: hey, you know initially I wanted to argue with some of your suggestions, but as I went all of them fit quite nicely :) | 16:06 |
| rogpeppe1 | dimitern: :-) | 16:06 |
| === tasdomas is now known as tasdomas_afk | ||
| fwereade_ | rogpeppe1, fwiw I think https://codereview.appspot.com/12097043/ is trivial; concur? | 16:13 |
| fwereade_ | rogpeppe1, State.pareTag | 16:13 |
| rogpeppe1 | fwereade_: yeah | 16:14 |
| fwereade_ | rogpeppe1, cheers | 16:14 |
| rogpeppe1 | fwereade_: oh, and i understand why you got weird linker errors too, i think | 16:14 |
| fwereade_ | rogpeppe1, o yes? | 16:14 |
| rogpeppe1 | fwereade_: it's because the tests fixtures themselves use State | 16:14 |
| rogpeppe1 | fwereade_: so, i think, external packages are linked against the non-test version of the code. | 16:15 |
| fwereade_ | rogpeppe1, hahaha, yes, that makes sense | 16:15 |
| rogpeppe1 | fwereade_: that's my current theory anyway - i haven't spent any time verifying it | 16:15 |
| fwereade_ | rogpeppe1, good enough for me, I resolved not to worry about it too much anyway ;) | 16:16 |
| rogpeppe1 | jtv: reviewed | 16:17 |
| rogpeppe1 | fwereade_: your review of https://codereview.appspot.com/12114043 appreciated too, please | 16:17 |
| fwereade_ | rogpeppe1, btw, https://codereview.appspot.com/11723043/ | 16:29 |
| rogpeppe1 | fwereade_: ha, thanks, i'd totally forgotten about that | 16:29 |
| * fwereade_ needs to be off, but will be back somewhat later to finish everybody's reviews | 16:39 | |
| sidnei | rogpeppe1: sorry, was out at lunch. was there any conclusion to what's the right fix? | 16:45 |
| rogpeppe1 | sidnei: your temporary workaround fix, i'm afraid, is to avoid using "." in attribute names | 16:46 |
| rogpeppe1 | sidnei: we need to sanitise the names used - the fix isn't hard, but we need to decide on an encoding scheme and actually do it | 16:46 |
| rogpeppe1 | sidnei: the issue, we're pretty certain, is because the names are used directly in mongo, which interprets the "." specially. | 16:47 |
| dimitern | rogpeppe1: updated again; one last look before I submit it? https://codereview.appspot.com/12034043 | 16:47 |
| rogpeppe1 | dimitern: looking | 16:48 |
| sidnei | rogpeppe1: hum, tricky but workable. i'd rather help you guys with implementing the sanitization though. | 16:49 |
| dimitern | rogpeppe1: btw had to merge trunk to resolve a conflict, but no other changes, despite the diff showing otherwise | 16:50 |
| rogpeppe1 | sidnei: you're welcome to propose a fix - the problem is in state/settings.go | 16:50 |
| sidnei | rogpeppe1: just need a decision on the encoding scheme, and then i can work from there | 16:52 |
| rogpeppe1 | sidnei: well, it needs to be reversible, and it needs to translate at least ".", "$" and probably leading digits (although i think we can probably just do "." and define the latter two as disallowed | 16:53 |
| rogpeppe1 | ) | 16:53 |
| rogpeppe1 | sidnei: "_"->"__"; "."->"_." ? | 16:55 |
| rogpeppe1 | sidnei: oh no, that's wrong | 16:55 |
| sidnei | :) | 16:55 |
| sidnei | what about we pick '.' -> ':', and disallow ':' | 16:56 |
| rogpeppe1 | sidnei: well, it depends if there are any current charms using "__" or "_." in attribute names | 16:56 |
| rogpeppe1 | sidnei: depends if : is allowed in mongo names | 16:56 |
| rogpeppe1 | sidnei: but that sounds like a good idea if it works | 16:56 |
| rogpeppe1 | sidnei: if you could do some investigation into that (and perhaps if any current charms define attributes with : in), that would be really useful | 16:57 |
| sidnei | rogpeppe1: if i read this correctly, they suggest using the unicode full width character replacements: http://docs.mongodb.org/manual/faq/developers/#faq-dollar-sign-escaping | 16:59 |
| rogpeppe1 | sidnei: well found; i was looking for something like that | 17:00 |
| rogpeppe1 | sidnei: i kinda prefer : though | 17:00 |
| sidnei | still need to escape the $ no? | 17:01 |
| rogpeppe1 | sidnei: i think i'd just disallow $ | 17:02 |
| sidnei | ok | 17:02 |
| rogpeppe1 | sidnei: although i'd like to do an audit of existing charms to check if anyone's using it | 17:02 |
| rogpeppe1 | sidnei: they might, of course, be using full-width $ and . :-) | 17:02 |
| rogpeppe1 | sidnei: i tell you what, let's just go with their suggestion. | 17:03 |
| rogpeppe1 | sidnei: it seems reasonable and it's easy to implement and quite intuitive | 17:04 |
| sidnei | okidoki | 17:04 |
| rogpeppe1 | sidnei: if you want to make the change, you might want to know about http://golang.org/pkg/strings/#Replacer | 17:04 |
| sidnei | was looking for that already :) | 17:05 |
| sidnei | rogpeppe1: do you still want to reject '$' or just do the replacement? | 17:11 |
| rogpeppe1 | sidnei: let's just do the replacement for the time being - at least there's a high assurance of backward compatibility doing it that way | 17:11 |
| rogpeppe1 | sidnei: we can make charm config names more strict later | 17:12 |
| sidnei | k | 17:12 |
| === natefinch is now known as natefinch-lunch | ||
| dimitern | rogpeppe1: does it look ok? | 17:22 |
| jtv | rogpeppe1: I see I'm too late to return the favour — not being very attentive late at night. Hope I can make up another day! | 17:29 |
| rogpeppe1 | dimitern: sorry, just had to go away for a little bit. i've a few comments, one mo | 17:32 |
| dimitern | rogpeppe1: sure, thanks | 17:34 |
| rogpeppe1 | dimitern: reviewed | 17:36 |
| dimitern | rogpeppe1: cheers | 17:39 |
| * rogpeppe1 has reached eod | 17:44 | |
| rogpeppe1 | g'night all | 17:44 |
| dimitern | rogpeppe1: night! | 17:44 |
| === tasdomas_afk is now known as tasdomas | ||
| === natefinch-lunch is now known as natefinch | ||
| ahasenack | guys, so I just deployed juju-core trunk on canonistack and then ran "juju deploy rabbitmq-server", straight from the store, no options | 18:18 |
| ahasenack | got this in the rabbit's unit logs: | 18:18 |
| ahasenack | error: --unit-name option expects "<service>/<n>" argument | 18:18 |
| ahasenack | and look at the upstart job: | 18:18 |
| ahasenack | exec /var/lib/juju/tools/unit-rabbitmq-server-0/jujud unit --data-dir /var/lib/juju --unit-name rabbitmq/server/0 --debug >> /var/log/juju/unit-rabbitmq-server-0.log 2>&1 | 18:19 |
| ahasenack | something mangled the rabbitmq-server/0 unit name to rabbitmq/server/0 | 18:19 |
| ahasenack | https://bugs.launchpad.net/juju-core/+bug/1206628 it is | 18:21 |
| _mup_ | Bug #1206628: Incorrect unit name in upstart job <juju-core:New> <https://launchpad.net/bugs/1206628> | 18:21 |
| dimitern | ahasenack: that's indeed a bug, stemming from the tags-to-names conversion I think, replacing - with /, etc. | 18:39 |
| ahasenack | dimitern: yeah, in state/api/deployer/unit.go there is such a conversion going on | 18:39 |
| ahasenack | func UnitTag(unitName string) string { | 18:39 |
| ahasenack | return unitTagPrefix + strings.Replace(unitName, "/", "-", -1) | 18:39 |
| dimitern | ahasenack: :) that's my doing actually, quite recent even | 18:40 |
| ahasenack | I like living on the edge :) | 18:40 |
| dimitern | ahasenack: i'll assign it to myself, thanks for the report | 18:40 |
| ahasenack | dimitern: thanks for taking care of it :) | 18:40 |
| dimitern | ahasenack: i will look into it tomorrow | 18:42 |
| ahasenack | ok | 18:42 |
| * dimitern reached eod as well | 18:42 | |
| dimitern | good night all! | 18:42 |
| ahasenack | dimitern: g'night | 18:42 |
| === tasdomas is now known as tasdomas_afk | ||
| thumper | morning | 21:03 |
| thumper | mgz: email update? | 21:44 |
| thumper | mramm: hey | 22:51 |
| mramm | hey | 22:52 |
| davecheney | mramm: arosales i've got a call with one of y'all now, right ? | 23:01 |
| arosales | davecheney, :-) | 23:01 |
| mramm | I scheduled right after arosales | 23:01 |
| arosales | davecheney, I think I have the privilege of being up first | 23:02 |
| mramm | so it would be him | 23:02 |
| davecheney | very well | 23:02 |
| davecheney | fire at will | 23:02 |
| davecheney | mramm: next please | 23:31 |
| mramm | joining | 23:32 |
| thumper | heh | 23:32 |
| thumper | gym time | 23:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!