/srv/irclogs.ubuntu.com/2013/02/20/#juju-dev.txt

=== wedgwood is now known as wedgwood_away
=== _thumper_ is now known as thumper
=== wedgwood_away is now known as wedgwood
=== wedgwood is now known as wedgwood_away
davecheneyniemeyer: OT, i met Benny Siegert last night00:27
davecheneyhe now works for google as an SRE00:27
davecheneyand was visiting Sydney00:27
niemeyerdavecheney: Benny.. hmm00:28
niemeyerdavecheney: I think I had a couple of interactions with him only in golang-nuts@00:29
niemeyerdavecheney: There was an event yesterday, right? How was it?00:29
davecheneyexcellent00:30
davecheneynumbers are improving every time00:30
davecheneywe had 50 people00:30
davecheneymore maybe00:30
davecheneythe room was packed00:30
thumperdavecheney: a go meetup?00:31
davecheneyyup00:32
davecheneythat was why I couldn't make the meeting last night00:32
davecheneyI run the sydney golang meetup group00:32
thumperah00:32
thumperI think I've just found a real use of the struct composition with methods...00:33
thumperthe alias struct in supercommand.go00:33
thumperit overrides Info00:33
thumperbut not the other methods00:33
thumperthose get resolved to Command?00:34
thumperI do have to be honest in thinking that this looks like bastardised inheritance00:34
davecheneythumper: it's much less than that00:34
davecheneyit is no inheritence at all00:34
thumperso how does it resolve the correct function calls?00:35
davecheneyalways at compilation time00:35
thumpersure, how00:35
davecheneyembedded fields are just syntactic sugar00:35
davecheneytype S struct { io.Reader }00:36
davecheneyvar s S00:36
davecheneys.Read is just sugar for s.Reader.Read00:36
davecheney^ one example00:36
thumperonly if s.Read doesn't exist?00:36
davecheneyyup00:36
thumperhmm00:36
* thumper pokes supercommand internals some mroe00:39
* thumper gets hit by python vs. go differences again00:43
thumperhmm...00:48
thumperdavecheney: how clever is go in matching similar names00:48
thumpereg. I have: type A struct {val Foo}00:48
thumperand a method00:48
thumperfunc bar() {00:48
thumperno...00:49
thumperfunc bar(val Foo) {00:49
thumpera = A{val: val}00:49
thumperwill it match those properly?00:49
thumperor do I have to rename something00:49
davecheneythumper: o_O00:55
thumperdavecheney: this is relatively normal python and c++00:55
davecheneydidn't undestand your example00:56
davecheneysay again, over00:56
thumperdavecheney: I have a function parameter that matches the name inside a structure00:56
thumperthat I'm trying to initialise00:56
thumperso I'm trying to create an A00:56
thumperwhere A.val == parameter val00:56
davecheneyyup, no problem there, they are different names00:56
thumperso A{val: val} is fine?00:57
davecheneysure00:57
thumpercoolio00:57
davecheneyval: isn't a symbol00:57
davecheneyit's the name of a struct member00:57
* thumper nods00:57
davecheneys/symbol/identifier00:57
davecheneywhen embedding you always have to disambiguate00:57
davecheneyie00:57
davecheneytype S struct { io.Reader }00:58
thumperdoes go have a set type?00:58
* thumper wants an ordered set00:58
davecheneyactually, that is a crap example00:58
davecheneyno, no set type00:58
davecheneyyou can use a map00:58
thumper:(00:58
davecheneymaps are not ordered00:58
davecheneymap iteration is random00:58
thumperno, but I want one00:58
thumperplz add to go 1.100:58
thumperset + ordered_set00:59
thumperand ordered_map00:59
thumper:)00:59
davecheneythe standard way is to extract the keys of a map00:59
davecheneysort them00:59
davecheneythen using that sorted slice00:59
davecheneyyou get the picture00:59
* thumper loves python00:59
thumperfor key, val in sorted(some_dict):00:59
davecheneyit is what it is00:59
thumperhmm.. missed the .items()01:00
* thumper nods01:00
* thumper will work around it01:00
thumperdavecheney: so... if a second parameter is passed to make([]string, size)01:02
thumperdavecheney: is the underlying array that size?01:03
thumperso meaning make([]string, 4) -> {"","","",""} ?01:03
thumperor is it actually empty with just enough space for 4 elements?01:03
* thumper feels like he needs to read the language spec again01:04
thumperhmm. found it01:05
thumpermake([]T, length)01:05
thumpermake([]T, length, capacity)01:05
thumperhmm01:05
thumperis foo := []string01:05
thumperan array or a slice?01:05
thumperactually: foo := []string{}01:06
davecheney[]slice01:06
davecheney[1234]array01:07
davecheneyvar foo []string -> nil slice, len and cap are 001:07
davecheneyfoo := []string{} -> non nil slice, len and cap are 001:07
davecheneyfoo := make([]string, 0) -> non nil slice, len and cap are 001:08
davecheneyif you are using reflect there is a difference between the 1st, and second and third arguments01:08
davecheneyotherwise there is no difference01:08
thumperso... what I want is foo := make([]string, 0, len(otherthing))01:09
thumperas I don't want anything initially in foo01:09
thumperbut I want to avoid unnecessary allocations01:09
thumperand I know at max there will be len(otherthing) elements01:10
thumpersound reasonable?01:10
davecheneysure01:10
thumperkk01:10
davecheneymost times we don't bother setting a capacity01:10
davecheneyjust01:10
davecheneyvar foo []string ; for _, v := range thing { foo = append(foo, v) }01:10
davecheney^ as an example01:11
thumperwhy?01:11
thumperhow often would the underlying array resize?01:11
thumperit just seems like good manners to me01:11
thumperperhaps I'm over thinking01:12
davecheneyi forget the exact mthod01:15
davecheneydoubling up to 1000 elements then +1000 I think01:15
davecheneyi can find out01:15
thumperso, adding 20 items causes how many allocations to an empty slice?01:17
thumperwith a default capacity of zero01:17
thumperit means initially you are getting quite a few, unless the 0 -> next is something like 1001:17
thumperthe default capacity specified for C++ std::vectors is 10 (by most implementations) for this exact reason01:18
thumperhmm...01:18
thumpernot doubling all the time was a cause of a bug I hit with solaris c++ std::stringstreams a long while back01:18
thumperit was iterative01:18
thumperand not expecting the amount of data I was throwing at it01:19
davecheneyI can check, i think it might start at 801:19
davecheneybut I am guessing01:19
davecheneyif you think it is important, then set the capacity when known01:19
davecheneybut avoid getting distracted by all the other places in the code where we do not do so01:19
thumperI only think it is important because I want to avoid unnecessary allocations01:19
thumperhowever01:20
thumperthere will likely be absolutely zero notice by a user01:20
thumperso I'll keep the code simple01:20
* thumper on the move again01:34
thumperbollocks, bollocks, bollocks02:55
* thumper hacks and slashes recent changes02:55
thumperdavecheney: back?03:31
thumperif I have a type of []string, and a method that takes (args ...string)03:31
thumperhow to I pass the former into the latter?03:31
thumperfound it03:34
davecheneyyup, varags syntax03:37
davecheneyfunc F(args ...string)03:37
davecheneyvar s []string03:37
davecheneyF(s...)03:37
thumperdavecheney: found it in the spec03:43
thumperquite a handy doc that03:43
thumperI'm liking how fast go compiles03:43
davecheneyyeah, that is nice03:43
thumperdavecheney: am I right in thinking that len(nil) == 0?03:46
=== gary_poster is now known as gary_poster|away
davecheneythumper: depends on the type of nil03:47
davecheneythat is capital T type03:48
davecheneyi'm not really sure03:48
davecheneylen of a nil slice or map is 003:48
thumperok, it is for a nil slice03:48
davecheneyin that case, yes, it len of a nil slice of map is 003:49
davecheneythe of map is new in 1.103:49
davecheneyno, sorry03:49
davecheneyit in 1.003:49
davecheneythere is another simplification for map in 1.103:49
thumperdavecheney: I have another branch, this one changes the way aliases for supercommands are handed04:02
thumperhandled04:02
davecheneynice04:02
thumperjust running all the tests now prior to proposing04:03
davecheneypropose!!04:03
davecheneyscrew that, proposing takes minutes04:03
thumpernow we get aliases shown in the generated help04:03
davecheneytests will be done by then04:03
thumper:)04:03
thumperit takes about 90s to run all the tests here04:03
thumperactually, I should time it again04:03
thumperI found myself after lunch futsing around too much with the aliases in the supercommand04:04
davecheneythe work stealing algo for running multiple tests packages could be better04:04
thumperin order to provide the right help04:04
thumperended up reverting it all04:04
davecheneybut butter still would not be taking 90 seconds for a test04:04
thumperthen changing the way it was done in a different branch04:04
thumperif you ask for help on an alias, now it will give you help on the actual command and say at the bottom it lists all the aliases04:05
thumperhmm lbox died on a launchpad api error04:06
davecheneyyeah, it does that04:08
davecheneyjust do it again04:08
davecheneyi've learnt to keep my commit messages04:08
davecheneyin a text file04:08
thumperlbox is proposing...04:12
thumperhttps://code.launchpad.net/~thumper/juju-core/command-aliases/+merge/14946104:13
thumperso... I'm on dinner tonight04:13
* thumper thinks04:13
thumperI think we decided to go out for my daughter's birthday04:13
thumperthat makes it easy04:13
thumperhttps://codereview.appspot.com/737004304:14
davecheneyhave a good one04:17
thumperI will04:29
thumperit is at my favourite resturant here04:29
thumpersoooo... good04:29
davecheneyhow fortuitus04:31
rogpeppemorning all08:20
rogpeppeevening davecheney :-)08:20
dimiternrogpeppe: morning08:20
davecheneyevening all08:24
dimiterndavecheney: hiya08:24
davecheneyrogpeppe: do you think we have the same set of issues that we had with bootstrapping precise from quantal08:26
davecheneythat we do for ppl wanting to do 386 -> amd64, and all the other permutations08:26
rogpeppedavecheney: in this case, we just want to use 386 all down the line08:26
rogpeppedavecheney: so i don't think the same problems arise08:27
rogpeppedavecheney: although there may well be things i don't know about08:27
davecheneyrogpeppe: i think there are wrinkles08:28
davecheneyfor example08:28
davecheney_most_ ec2 instances are amd6408:28
davecheneyexcept for standard (?) and micro08:28
davecheneyso i think it is more compliated than saying 'i want this environment to have amd64 quantal machines'08:29
fwereadedavecheney, rogpeppe: this question is uppermost in my mind08:29
fwereadedavecheney, rogpeppe: and I *think* it's ok08:29
davecheneyfwereade: my initial response is to say 'juju only makes amd64 machines'08:29
davecheneyand ignore the arch on the host machine08:29
fwereadedavecheney, certainly we must ignore the arch on the host08:30
davecheneyso bootstraping from anything, osx runing on arm produces amd64 machines08:30
davecheneyfwereade: then that might solve rogpeppe 's question08:30
fwereadedavecheney, I think that the tighest constraint we have is what arches we have the tools built for08:30
davecheneyfwereade: i'd be relieved to say, for the future that we can predict, amd64 is all we do08:30
rogpeppedavecheney: what about --upload-tools ?08:30
davecheneyrogpeppe: fuk08:31
davecheneydouble shit08:31
rogpeppedavecheney: that's my motivating case at the moment08:31
rogpeppefwereade: mornin' BTW. hope you're feeling a little better.08:31
fwereadedavecheney, I don't think there's a compelling reason to drop i386 support on ec208:31
fwereaderogpeppe, heyhey, pretty crappy tbh, I will be off and on08:32
davecheneyfwereade: apart from it being a wrinkle we don't need08:32
rogpeppedavecheney: i'm not sure i see the problem.08:32
rogpeppedavecheney: currently the logic chooses the image based on the arch and series of the tools found.08:32
davecheneyrogpeppe: it's just more perumutations that we don't need righ tnow08:32
rogpeppedavecheney: i *think* all we need to do is provide a mongo db binary and it'll just work08:33
davecheneyrogpeppe: i don't think we need to do that08:33
davecheneywe should always request an amd64 image08:33
davecheneyif it happens that the development tools are 38608:33
rogpeppedavecheney: not if the caller wants to upload tools08:34
davecheneythat isn't a problem on linux08:34
rogpeppedavecheney: and have them run on the bootstrap machine08:34
davecheneyyeah, that works fine08:34
davecheneyjust clamp the AMI request to amd64, job done08:34
rogpeppedavecheney: ah, because you can always run 386 binaries ok08:35
* dimitern rebooting08:35
davecheneyyeah08:35
davecheneygoing the other way, amd64 tools on a t1.micro will be sadface08:35
davecheneybut that is a bridge to cross at another time08:35
fwereadedavecheney, is that any worse than *anything* on a t1.micro? ;p08:35
davecheneyexactly08:36
davecheneyit's not the way the market is heading08:36
rogpeppedavecheney: i'm not really sure i see the motivation though. we're going to need different mongo binaries sooner or later. why not just upload 'em now and avoid the special-case hacks in the code?08:36
davecheneyrogpeppe: --upload-tools is the hack08:36
davecheneyi'm fine with hackig it some more08:36
davecheneyrogpeppe: is there an issue for this work ?08:36
rogpeppedavecheney: yeah, wait a mo08:37
rogpeppedavecheney: https://bugs.launchpad.net/juju-core/+bug/113020908:37
_mup_Bug #1130209: we need a mongodb compiled for i386 <juju-core:New> < https://launchpad.net/bugs/1130209 >08:37
rogpeppedavecheney: but that's fairly specific - i assumed it was the right solution08:38
davecheneyrogpeppe: mind if I rename that issue to what I see is the problem ?08:38
rogpeppedavecheney: ok08:39
rogpeppedavecheney: which is?08:39
davecheney"juju should always request an amd64 AMI image"08:41
davecheneyactually, that will probably mean we can't boot t1.micros08:41
davecheneybut whatever08:41
fwereadedavecheney, I *think* that we can start any ec2 instance with AMD6408:42
fwereadedavecheney, but I'm not quite convinced this is an ok shortcut08:42
davecheneyfwereade: why not ?08:43
fwereadedavecheney, haven't people already used juju on ARM machines? even if they don't run the state server?08:43
rogpeppefwereade: i've made some replies to https://codereview.appspot.com/7326052/08:43
davecheneyfwereade: only as a client08:44
davecheneymy view of the crystal ball shows only environments containing amd64 machines08:44
davecheneywhen highbank or calexeda come asking, we fix the problem then08:44
fwereadedavecheney, what about the arm stuff that had to be added to python juju for maas?08:44
rogpeppefwereade: i'm not sure what to do. i really don't have time to fix all the bugs currently - i can abandon the CL and just leave as an example for the GUI folks if you'd prefer.08:45
davecheneyfwereade: in the spirit of delivering 13.04, we have to leave the goal posts in the ground08:45
fwereadedavecheney, I do not believe I am the person trying to move them...08:45
rogpeppedavecheney: what *is* the problem with just uploading 386 mongo binaries, BTW?08:45
davecheneyrogpeppe: well, 1, we don't have those binaries, so that is work08:46
rogpeppedavecheney: i pointed you at some08:46
davecheney2, getting the deps in the archive is going to be an amazing ballache, so i'd rather no x2 it08:46
rogpeppedavecheney: i don't understand - which deps in which archive?08:46
davecheneyrogpeppe: with respect, I have proposed a solution that has a low cost now, and a known cost in the future08:47
davecheneywe don't have to gold plate this08:47
davecheneywe just have to make it work on jam's dev machine, right ?08:47
rogpeppedavecheney: if i'd had the credentials, i could have fixed this with a single s3 upload last night08:47
davecheneyrogpeppe: how can I send them too you ?08:48
davecheneyi'm more than happy for someone else to fix this08:48
rogpeppedavecheney: i have the ones you gave me before, but they don't seem to work any more08:48
davecheneyyeah, niemeyer reset them a few times trying to get the permission junk working08:49
rogpeppedavecheney: ah08:49
davecheneyrogpeppe: email me your public key08:49
davecheneyi'll bootstrap a machine08:49
davecheneyand leave them there for you08:49
fwereaderogpeppe, the issue with that CL is that moving code from one wrong place to a different wrong place is of limited utility... is there some other command that is a bit more robust and doesn't need to be reimplemented inside state?08:51
rogpeppefwereade: well, surely the *place* is right - it's just the code is wrong?08:51
fwereaderogpeppe, the code cannot be correct while it's in that place, though, can it?08:52
rogpeppefwereade: the code cannot be correct in any place!08:52
rogpeppefwereade: 'cos it's buggy08:53
fwereaderogpeppe, yeah, but the bugs are intractable while it's outside the state package08:53
rogpeppefwereade: do you think we should just delete set and get support because they're currently wrong?08:53
fwereaderogpeppe, I think that picking something that's wrong as a demonstration of how to do something right is problematic08:54
rogpeppefwereade: i agree that stuff needs to move into the state package, but i don't think i want to say to the gui folks - "yes you can make access to the command functionality through the API but you have to fix all our current bugs as you do so"08:54
fwereaderogpeppe, ok, but... what will statecmd be, then, if not just a holding area for buggy code, soon to be replaced by better state API calls?08:58
fwereaderogpeppe, this feels like inserting an extra step... you don't have to fix the bugs, but I don't see what the benefit is of adding another layer08:59
rogpeppefwereade: the benefit of the extra layer is at least we're not duplicating the buggy code09:00
rogpeppefwereade: because it needs to be called from two places now09:00
fwereaderogpeppe, er, what? why would we be duplicating it if we put it in one place, ie in state?09:01
fwereaderogpeppe, both those places use the state API though, right?09:01
rogpeppefwereade: the idea is that the functions in statecmd are *directly* equivalent to the command line commands, with all the parameter checking that implies.09:03
rogpeppefwereade: i thought about putting them in state too09:03
rogpeppefwereade: it depends whether we feel that almost all the code in juju/cmd should actually be done in state.09:04
rogpeppecmd/juju09:04
fwereaderogpeppe, well that case obviously has to be; and offhand I can't think of one that shouldn't, can you?09:06
rogpeppefwereade: so what should i say to the GUI folks? hold off while we refactor cmd/juju ?09:07
fwereaderogpeppe, I would say "put it in state, like this"... maybe I'm not understanding some of the constraints?09:08
fwereaderogpeppe, blast, sorry, need a quick break, can we pick this up a bit later?09:09
rogpeppefwereade: ok09:09
fwereaderogpeppe, hey, sorry to abandon you; I think I might be returning to bed for a while again soon, but we should finish this09:40
rogpeppefwereade: shall we G+?09:40
fwereaderogpeppe, sgtm, will you start?09:40
rogpeppefwereade: ok09:40
rogpeppefwereade: ha, i've made a hangout, but the window has disappeared09:43
* fwereade tries09:43
fwereadejam, ping: https://codereview.appspot.com/7307128/patch/1/611:00
fwereadejam, for your first comment, I thought it was covered by "// Environ constraints are not available before initialization." just above?11:01
fwereadejam, and for the second: yes to the first bit, clarify please to the seconds11:01
fwereadejam, I'm still pretty flaky today so am appearing and disappearing at random, just answer whenever and I'll see it when I see it11:02
=== teknico_ is now known as teknico
=== edamato is now known as edamato-afk
dimiternjam, mgz: standp11:32
=== mohits1 is now known as mohits
rogpeppefwereade: i already have two LGTMs on this, but i'd really like your input too, as you've been following this stuff: https://codereview.appspot.com/7314116/12:14
=== edamato-afk is now known as edamato
fwereaderogpeppe, am I right in thinking there's no api.IsNotFound?12:29
fwereaderogpeppe, also, who should I be talking to to figure out the deal with yaml-in-the-gui?12:32
fwereaderogpeppe, because I'm still pretty strongly -1 on the two-parallel-paths-not-checked-on-the server bit12:33
fwereadedimitern, rogpeppe, TheMue: would one of you do a quick review on https://codereview.appspot.com/7331053/ please? it's near-as-dammit trivial and already has one LGTM12:49
rogpeppefwereade: hmm, sorry missed your remarks above12:50
rogpeppefwereade: from the top:12:50
rogpeppefwereade: yes, there's no api.IsNotFound - it's spelt api.ErrCode() == api.CodeNotFound instead12:51
fwereaderogpeppe, how about something more like api.ErrHasCode(err, api.CodeNotFound), perhaps?12:52
fwereaderogpeppe, just bikeshedding really, but I thought that IsNotFound was pretty handy12:53
rogpeppefwereade: in #juju-gui, bac or gary_poster are two possibilities12:53
fwereaderogpeppe, cool, ty12:53
rogpeppefwereade: in ec2, i started with HasCode, but it turns out it's nicer to have a function that returns the error code.12:53
fwereaderogpeppe, ok, fair enough on that, not sure I really like my suggestion12:54
rogpeppefwereade: for one thing, in tests, c.Assert(api.ErrCode(err), Equals, api.CodeNotFound) gives a nicer error message on failure12:54
rogpeppefwereade: also, you can do switch code := api.ErrCode(err); code {12:54
fwereaderogpeppe, ok, sgtm, thanks :)12:55
rogpeppefwereade: np12:55
rogpeppefwereade: FWIW i don't think the two parallel paths thing is really a problem - i just documented that Config overrides Options.12:55
rogpeppefwereade: but there's an issue around that that i was discussing with niemeyer yesterday (in your absence)12:56
rogpeppefwereade: which is that yaml is strictly more expressive than the options in this context, because yaml can express null, but options cannot, only an empty string.12:56
fwereaderogpeppe, feels like unnecessary nastiness with no real benefit to me... if we *have* to have the yaml path I think it's better to convert to yaml at the CLI end than to overload the params12:57
fwereaderogpeppe, isn't an empty string "fall back to the default" in this context?12:57
rogpeppefwereade: if only js had decent yaml support... (if only yaml wasn't insane...)12:57
fwereaderogpeppe, ha, yeah12:57
rogpeppefwereade: yes, but it could also be "set this to the empty string"12:58
rogpeppefwereade: and in yaml you can express that difference12:58
rogpeppefwereade: my inclination is to always make an empty string equivalent to null12:58
rogpeppefwereade: then the two paths are equivalent12:58
fwereaderogpeppe, yeah, I think we kinda decided that a while ago, but that might just be in informal inference I made12:59
rogpeppefwereade: FWIW niemeyer thought that it was reasonable to have yaml be more expressive.12:59
TheMuefwereade: You've got a +1.13:00
rogpeppefwereade: BTW if it wasn't for the js stuff, i would have no hesitation at all about dropping the yaml path13:00
TheMuefwereade: And thx for your review, the change is proposed again.13:00
fwereaderogpeppe, in that case I think we should drop Options -- it is more important to me that we drop one of them rather than it sepcifically be the yaml13:00
fwereadeTheMue, tyvm, I'll take a look in a bit13:00
rogpeppefwereade: i just had a little chat in #juju-gui - here's the result: http://paste.ubuntu.com/1690301/13:05
rogpeppefwereade: the thing is that if we drop Options, then, when they're not doing file upload, the gui will have to create well-formed yaml from the given options. perhaps that's not a problem, but i don't have an issue with there being two ways to specify the same thing, as long as the interaction between the two is well defined.13:07
rogpeppefwereade: alternatively, we could have two entry points: ServiceSet and ServiceSetYAML13:08
rogpeppefwereade: under the surface they'd invoke the same thing13:08
fwereaderogpeppe, I think I'd rather separate them like that if nothing else13:09
=== gary_poster|away is now known as gary_poster
rogpeppefwereade: ok, although i still don't really understand the problem13:09
fwereaderogpeppe, or... they could just JSON it in the GUI, and that's still valid YAML, right?13:10
fwereaderogpeppe, so long as we handle ""/null consistently13:10
rogpeppefwereade: true. i suppose my ickiness head goes "json-in-json-string, yeuch"13:10
fwereaderogpeppe, I think that with the YAML requirement we're already way over that line13:11
rogpeppefwereade: that's only required if you want to upload yaml. i'd prefer not to pollute the clean path because of that requirement13:12
fwereaderogpeppe, can you live with two separate calls?13:14
fwereaderogpeppe, that seems to me like the least polluting approach possible ;)13:16
rogpeppefwereade: yeah, i'll do two separate calls.13:17
fwereaderogpeppe, cool, tyvm13:25
fwereadeTheMue, I'm wondering about the 301s on paths with ..13:26
fwereadeTheMue, surely if anything's a 403, it's attempts to read/write outside the path?13:26
fwereadeTheMue, and a prefix of "../" acting the same as no prefix is a bit weird as well I think13:27
fwereadeTheMue, when listing I mean13:27
TheMuefwereade: I've wondered too and it took me some time to discover it.13:29
TheMuefwereade: It's a feature of the http daemon of Go.13:29
TheMuefwereade: It resolves relative paths and sends a redirect to the client.13:29
TheMuefwereade: And I also wondered about why using it with a pattern doesn't lead to a 301. Strange.13:30
fwereadeTheMue, hmm, where does it redirect to?13:32
TheMuefwereade: I have to check now that the URL doesn't contain the environ name anymore.13:33
TheMuefwereade: Before it has been from /environname/../foo to /foo13:34
TheMuefwereade: But only a redirect, I client has to interpret it itself.13:34
fwereadeTheMue, hmm, would you look into that please? it should not have been doing that, that gets access to the files in the foo environment13:35
TheMuefwereade: Now as it is change (w/o the environment as part of the URL) this can't happen. But the access when knowing a path or the port has already been open before.13:36
TheMuefwereade: Right now we don't do any authentication on the storage.13:36
fwereadeTheMue, so where does localhost:60006/../foo redirect to?13:36
TheMuefwereade: I have to look. But as I said: the redirect is no access. It's only a transformation of the URL (w/o validation if a path exists) and sending it to a possible client).13:38
TheMuefwereade: If that does then react on it and does a request and the file doesn't exists it gets a 404.13:38
fwereadeTheMue, fwiw I think you'll find the cli will probably be losing direct environ access in the near future, so the security point may be moot13:39
TheMuefwereade: But give me some minutes, now I'm interested in that redirect too.13:39
fwereadeTheMue, but nonetheless I would rather have a PUT to ../wherever/blah *fail* rather than just put *somewhere* ;)13:39
fwereadeTheMue, thanks13:39
TheMuefwereade: Sorry, the Go server works differently. I have to look if it provides a kind of hook to get involved in the resolving.13:40
TheMuefwereade: And security has nothing to do with the generated redirect, here we should introduce a real authentication feature.13:41
TheMuefwereade: At least basic auth.13:41
fwereadeTheMue, what scenarios are we protecting against here?13:43
TheMuefwereade: One moment, testing.13:44
fwereadeTheMue, I guess multiple users running local jujus on the same machine... but even then, each of them must be able to become root in order to start an env, right?13:45
fwereadeTheMue, but then people who can't start an env could still look inside and/or write to it, so, I guess, yeah: we should have some auth13:46
TheMuefwereade: GET /../* is directly delivered (no redirect, as I said) as /*13:46
fwereadeTheMue, ok, that's fair enough I suppose; but GET /../foo redirects?13:47
TheMuefwereade: Yeah, the storage backend is totally open right now, so beside using our own tool any wget/curl/scripting can simply access it in any way. And that's not good in the long run.13:48
fwereadeTheMue, it *is* a dev tool, not a production system... and, hold on, can we not restrict it to connections from localhost?13:49
TheMuefwereade: /../foo* to /foo*13:49
fwereadeTheMue, so that one's a redirect?13:50
TheMuefwereade: But as said, it is NO redirect. The redirect is in PUT (funnily) and has also been when we used the environ name in the path.13:50
fwereadeTheMue, ok, sorry: GET never redirects, but PUT and DELETE do?13:51
TheMuefwereade: Restriction by IP should be possible, but not localhost, but the IP of the LXC containers.13:51
fwereadeTheMue, it's not clear that we ever want something in a container to use it as a Storage13:52
TheMuefwereade: Hehe, ok, step by step.13:52
mgzright, I was wondering about access control on the local provider storage too13:52
rogpeppefwereade: any chance of a review of https://codereview.appspot.com/7314116/ ? (it sounded like you were looking at it earlier - perhaps you just didn't publish your comments?)13:52
fwereadeTheMue, when should a container be able to get a Storage to work with?13:52
TheMuefwereade, mgz: Please seperate those discussions, first redirect, later security.13:53
TheMuefwereade: Once again, step by step.13:53
mgzTheMue: I'll try to leave comments on the review13:53
rogpeppemgz: eventually, we could probably use admin-secret for security, if we cared much.13:53
mgzrogpeppe: it's probably less security and more accidental clashing you care about for local13:54
TheMuefwereade: First approach has been to use a URL like host:1234/<<environ>>/<<name>>.13:55
TheMuefwereade: In that case a ".." leads to a redirect.13:55
rogpeppemgz: i don't think accidental clashing is a problem necessarily - we can store the address of the storage in the environ's directory.13:55
TheMuefwereade: Now we use host:1234/<<name>>.13:55
TheMuefwereade: Here /../<<name>> doesn't lead to a redirect, but the ".." vanishes.13:56
fwereadeTheMue, ok, with all verbs?13:56
TheMuefwereade: But in case of a PUT, the Go HTTP daemon returns a 301.13:56
fwereadeTheMue, ah, ok, sorry13:56
TheMuefwereade: Needed some time to get behind it as it screwed up my expectations and tests. :D13:57
fwereadeTheMue, ...and *that* redirects to..?13:57
fwereadeTheMue, /<<name>> ?13:57
TheRealMuefwereade: But again, it doesn't deliver ata, it does no existence check, nothing.14:00
fwereadeTheRealMue, sure, that wasn't really on my mind -- I just wanted to know *what* it was doing :) maybe it would be good to document the details of what's going on under the hood, in the tests?14:02
TheRealMuefwereade: Yeah, PUT /../foo redirects to /foo.14:02
TheRealMuefwereade: I'll document it, yip.14:02
fwereadeTheRealMue, tyvm14:02
TheRealMuefwereade: It's simply a "if contains("..") { transformAndRedirect(url) }. ;)14:03
=== TheRealMue is now known as TheMue
* rogpeppe goes to lunch14:08
niemeyerYo14:24
=== mohits1 is now known as mohits
=== wedgwood_away is now known as wedgwood
dimiternniemeyer: hey14:31
fwereadeniemeyer, heyhey14:39
fwereaderogpeppe, mramm, TheMue: kanban?14:57
rogpeppefwereade: in 2 mins, no?14:57
TheMuefwereade: Yep, in 2 mins.14:57
fwereaderogpeppe, what's the deal with 208-simplify-config-tests? did that go anywhere, or was another variant chosen, or what?14:58
rogpeppefwereade: ah, i was reminded of that this morning. i haven't yet addressed jtv's final comment on that CL14:58
bacrogpeppe: there are naming differences between what you landed as your first branch and the second branch that is under review, e.g. ServiceSetParams vs. SetConfigParams.  which is the one you'll be using?15:17
fwereaderogpeppe, I would favour longer but more painfully explicit names, if possible15:30
fwereaderogpeppe, sorry, my comments on the second don't apply so well in the light of the decusuins re the first15:31
rogpeppefwereade: i thought ServiceSet was ok15:31
rogpeppefwereade: (given that we've got "juju set")15:31
rogpeppefwereade: but you'd prefer ServiceSetConfigParams?15:31
fwereaderogpeppe, ah, hmm, I think consistency there is actually quite good15:32
rogpeppefwereade: well, sure, but i'd have ServiceSetConfig to match15:32
rogpeppefwereade: i just thought that was on the verge of unnecessarily verbose15:32
fwereaderogpeppe, yeah, indeed, I am newly taken with the benefits of ServiceSetParams/EnvironSetParams(?)15:33
rogpeppefwereade: great, thanks15:33
rogpeppebac: i changed the names as suggested in the review15:33
rogpeppebac: the second branch will change to match - i still need to do some work on it as per discussions before landing it15:34
bacrogpeppe: ok, so the names in the landed branch are preferred.  thanks.15:35
* TheMue has a late lunch today, biab15:39
fwereaderogpeppe, btw, is there a bug in LP for that `juju-log "--lol screw you"` issue?15:48
rogpeppefwereade: no, gimme 30s15:48
fwereaderogpeppe, would you assign it to tim please, and suggest the SetFlags tweak we discussed?15:49
rogpeppefwereade: k15:49
rogpeppefwereade: hmm, do you know where the implementation of the jujuc commands is in the python?15:56
rogpeppefwereade: oh, found it15:57
fwereaderogpeppe, sorry I missed you15:57
rogpeppefwereade: i can't see how the "we don't interpret flags" thing is consistent with the fact that there's a -l flag to juju-log15:58
rogpeppefwereade: i can't see how that works in the python15:58
rogpeppefwereade: (i'm looking in juju/hooks/commands.py, BTW)15:59
fwereaderogpeppe, I would guess that the parser just ignores the stuff it doesn't recognise and passes it on in the args15:59
fwereaderogpeppe, what does it use: optparse?15:59
rogpeppefwereade: argparse16:00
fwereaderogpeppe, yeah, I think that has parse_known_args?16:01
rogpeppefwereade: it doesn't use that method16:02
rogpeppefwereade: it just uses parse_args16:02
* rogpeppe manages to see through *some* of the python magic16:03
fwereaderogpeppe, ah, wait, it gets a bunch of stuff from juju/hooks/cli.py as well I think16:03
rogpeppefwereade: yeah, that's the place that calls parse_args16:04
rogpeppefwereade: in the __call__ magic method16:04
mrammanybody have a link to tim's gap analysis doc?  I can't seem to find it in my google drive.16:04
rogpeppefwereade: hmm, i think this might be magic too:         self.parser.add_argument("message", nargs="+")16:05
rogpeppefwereade: perhaps that "+" signifies "anything remaining"16:05
fwereaderogpeppe, very probably16:05
rogpeppefwereade: so "juju-log -l INFO foo" will send a "foo" message at info level, but "juju-log -z foo" will log "-z foo" at normal level16:06
rogpeppefwereade: if that's true, it's really not very nice and breaks all our arg parsing rules.16:06
fwereaderogpeppe, sorry, my dad just arrived from england, was saying hello16:12
rogpeppefwereade: np16:12
fwereaderogpeppe, I agree that that is not very nice16:12
rogpeppefwereade: am just trying to understand argparse16:12
rogpeppefwereade: "+" just says any remaining args - it still seems to give an error when a flag isn't recognised16:12
fwereaderogpeppe, hmm, maybe it has special handling for "--" followed by "something weird"16:13
rogpeppefwereade: maybe - i'll try that next16:13
rogpeppefwereade: yup, that's right. yulk16:15
rogpeppefwereade: here's my testbed: http://paste.ubuntu.com/1691047/16:16
rogpeppefwereade: sample: http://paste.ubuntu.com/1691052/16:16
rogpeppefwereade: looks like if you've got a space in the arg it's treated as if it can't be a flag16:17
fwereaderogpeppe, ok, I guess that makes sense, but still, eww :)16:17
rogpeppefwereade: no, that's not true either16:18
rogpeppefwereade: if there's a space in the arg, it's treated as a flag *only* if the prefix doesn't match a flag16:18
fwereaderogpeppe, wait what? I don;t think that fits with what you showed me16:19
fwereaderogpeppe, treated as an arg?16:19
rogpeppefwereade: so "tst.py '-z arg'" goes through ok but "tst.py -z ' arg' fails16:19
rogpeppefwereade: which seems so so wrong to me16:20
fwereaderogpeppe, I dunno, it actually seems pretty sane... but it's annoying to have to replicate all the magic16:20
fwereaderogpeppe, it's not *nice* but it is making an effort to do the Right Thing even in the face of crazy args like "--> roflcopter"16:21
rogpeppefwereade: it seems wrong to me because in general i'd think that -z$arg and -z "$arg" should be handled exactly the same16:21
fwereaderogpeppe, yeah, reasonable perspective16:21
rogpeppefwereade: what if the create arg as "-lINFO" ?16:21
rogpeppes/create/crazy/ s/as/was/16:21
fwereaderogpeppe, well it *is* meaningless anyway because we dropped all the logging levels :/16:22
rogpeppefwereade: ha16:22
fwereaderogpeppe, gtg for a bit, back later16:22
rogpeppefwereade: ok16:22
rogpeppefwereade: https://bugs.launchpad.net/juju-core/+bug/113077116:48
_mup_Bug #1130771: juju-log is not compatible with python version <juju-core:New> < https://launchpad.net/bugs/1130771 >16:48
fwereaderogpeppe, responded17:10
TheMueniemeyer: Your ubuntufinder.com is quite cool.17:11
rogpeppefwereade: i wonder if the argparse semantics are actually documented anywhere17:11
dimiternniemeyer: yeah, pretty cool UI with these rolling radio buttons / shelves :)17:14
rogpeppefwereade: i had a go at fixing that live test failure yesterday, and got some way, but it still fails. does this look reasonable to you? http://paste.ubuntu.com/1691273/17:17
fwereaderogpeppe, hmm, that looks like it ought to work17:18
rogpeppefwereade: unfortunately the provisioner never actually shuts down the instance. i'm wondering if i should remove the machine too.17:18
fwereaderogpeppe, no, that's the provisioner's job... does it really not do it? I could have sworn it did17:19
rogpeppefwereade: yeah, i thought so too17:19
fwereaderogpeppe, if not then, well, this is why it was bad to invent our own shutdown process in those tests ;p17:19
rogpeppefwereade: BTW, i *think* that Machine.Destroy should not require a Refresh if the last view of the machine had assigned units.17:19
rogpeppefwereade: agreed17:20
fwereaderogpeppe, we should try regardless of the latest known state? not convinced, but go on17:20
rogpeppefwereade: yeah, because it's what's actually out there in the state that's important AFAICS17:21
rogpeppefwereade: in the above loop, there's no real reason we need to keep on refreshing while trying Destroy periodically17:21
fwereaderogpeppe, I would think it more usual to watch, refresh on change, and then use that -- recent -- state as a basis for making further decisions on what to do17:22
rogpeppefwereade: in this case, there's nothing to watch17:22
rogpeppefwereade: or... maybe there is17:23
rogpeppefwereade: well, anyway, the in-memory state has no bearing on whether we can or can't call Destroy17:23
fwereaderogpeppe, a machine watch will observe those changes17:23
rogpeppefwereade: yeah17:23
fwereaderogpeppe, it has plenty of bearing -- once we've figured out whether it's a sane request, we decide whether or not to even call anything, all based on in-memory state17:25
rogpeppefwereade: why should we? the in-memory state doesn't tell us anything about whether the machine has units *now*, no?17:25
mgzbah, why did I type images, I meant instance-types17:26
mgzterminology is hard17:26
rogpeppefwereade: hmm, except, i guess, when there are *no* units and the machine is dying17:26
fwereaderogpeppe, no, but if you're interested we'll tell you when it changes... why accommodate impatient hammering of the API17:26
rogpeppefwereade: i'm not sure why {wait for change; refresh; destroy} is better than {wait for change; destroy}17:27
rogpeppefwereade: it certainly surprised me when i discovered it was necessary.17:27
fwereaderogpeppe, because if you didn't refresh then you obviously have no basis for assuming it's destroyable ;p17:28
rogpeppefwereade: i know it changed :-)17:28
rogpeppefwereade: the overhead of calling destroy is probably not far off that of calling refresh. or... maybe it is actually much cheaper in fact.17:28
fwereaderogpeppe, define cheap... destroy will potentially lock the service document, won't it?17:30
rogpeppefwereade: yeah, i guess so17:30
fwereaderogpeppe, ah wait that's only for a unit, doesn't apply17:31
rogpeppefwereade: oh yes, not for a machine17:31
fwereaderogpeppe, I somehow switched to thinking about units17:31
rogpeppefwereade: anyway, i think of mutating operations on the state as write-through. i found it strange that one failed only because of in-memory state.17:31
rogpeppefwereade: (even though the actual state would have allowed the operation)17:32
fwereaderogpeppe, I've been thinking of mutating operations on the state as scary, and preferring to avoid them unless I'm as sure as I can be that they'll go through17:32
fwereaderogpeppe, because it is not easy to determine by inspection just what docs will be involved in a given txn, and how many other txns get clipped by it17:33
rogpeppefwereade: it's a pity we can't even set a machine to Dying when it has units - then having no units with a dying machine would be definitive17:34
fwereaderogpeppe, that's a known bug that we officially do not care enough about to fix by 13.0417:35
fwereaderogpeppe, if the provisioner's not actually cleaning up the machine, that is an issue of vastly greater magnitude ;)17:36
rogpeppefwereade: true 'nuff17:37
frankbanhi all, does the current juju-core trunk support default-series? I have precise in my env configuration, but nodes are created using quantal.17:48
rogpeppefwereade: do you know about the above question?17:49
niemeyerTheMue, dimitern: Cheers!17:50
rogpeppefwereade: first watcher in the API implemented: https://codereview.appspot.com/7390043/17:51
fwereaderogpeppe, heh, you would have been the person I asked17:51
fwereadefrankban, so the charm you're using is definitely a precise one?17:52
rogpeppefwereade: darn. i should really know :-)17:52
fwereaderogpeppe, I am up to my elbos in that stuff at the moment so the behaviour will doubtless change somewhat over the next couple of says17:52
fwereaderogpeppe, but it does *kinda* look like every machine is started with the series/arch of the machine that launches it :/17:52
rogpeppefwereade: it's started with the series/arch of whatever tools it can find, i think17:53
frankbanfwereade: yes, it's the juju gui charm (deployed from local repository). however, the bootstrap node is also quantal17:53
rogpeppefwereade: anyway, gotta go. we're packing the car up, off to the Lakes for some snowy hills17:54
fwereaderogpeppe, enjoy17:54
bacbye rogpeppe.  you around tomorrow?17:54
rogpeppefwereade: will do. have a great rest of week. should be in contact sporadically by email17:54
rogpeppebac: no, sorry. away for rest of week.17:54
bacrogpeppe: have fun!17:55
rogpeppebac: will try me best :-17:55
rogpeppe)17:55
fwereadefrankban, ok, that just sucks profoundly... but, yeah, I don't see how that's meant to actually work17:56
fwereadefrankban, it seems that the machine we start is to all intents and purposes random17:57
fwereadefrankban, good thing I am working on that right now17:57
frankbanfwereade: cool, thanks17:58
rogpeppesee y'all monday!17:59
bachi, i've put a small branch up for review at https://codereview.appspot.com/7401043/21:22
thumperbac: looks good to me, but I have no confidence in my ability to review it yet21:29
thumperbac: particularly to know if it works or is even right :)21:30
thumperQ) an uninitialized string in a struct has a value of "" ?21:34
bacthumper: yes21:39
thumpercool...21:39
bacthumper: thanks for looking.  since it is just code reorg and the previously existing tests still pass that says something (though the tests aren't much)21:40
thumperhopefully within a week or so I'll feel happier commenting on merge proposals21:40
bacthumper: is the requirement two reviews?21:42
thumperyeah21:42
=== wedgwood is now known as wedgwood_away
=== wedgwood_away is now known as wedgwood
davecheneythumper: will review your CL in two secs22:40
thumpersure22:40
* thumper afk for lunch22:46
=== wedgwood is now known as wedgwood_away

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