[00:27] <davecheney> niemeyer: OT, i met Benny Siegert last night
[00:27] <davecheney> he now works for google as an SRE
[00:27] <davecheney> and was visiting Sydney
[00:28] <niemeyer> davecheney: Benny.. hmm
[00:29] <niemeyer> davecheney: I think I had a couple of interactions with him only in golang-nuts@
[00:29] <niemeyer> davecheney: There was an event yesterday, right? How was it?
[00:30] <davecheney> excellent
[00:30] <davecheney> numbers are improving every time
[00:30] <davecheney> we had 50 people
[00:30] <davecheney> more maybe
[00:30] <davecheney> the room was packed
[00:31] <thumper> davecheney: a go meetup?
[00:32] <davecheney> yup
[00:32] <davecheney> that was why I couldn't make the meeting last night
[00:32] <davecheney> I run the sydney golang meetup group
[00:32] <thumper> ah
[00:33] <thumper> I think I've just found a real use of the struct composition with methods...
[00:33] <thumper> the alias struct in supercommand.go
[00:33] <thumper> it overrides Info
[00:33] <thumper> but not the other methods
[00:34] <thumper> those get resolved to Command?
[00:34] <thumper> I do have to be honest in thinking that this looks like bastardised inheritance
[00:34] <davecheney> thumper: it's much less than that
[00:34] <davecheney> it is no inheritence at all
[00:35] <thumper> so how does it resolve the correct function calls?
[00:35] <davecheney> always at compilation time
[00:35] <thumper> sure, how
[00:35] <davecheney> embedded fields are just syntactic sugar
[00:36] <davecheney> type S struct { io.Reader }
[00:36] <davecheney> var s S
[00:36] <davecheney> s.Read is just sugar for s.Reader.Read
[00:36] <davecheney> ^ one example
[00:36] <thumper> only if s.Read doesn't exist?
[00:36] <davecheney> yup
[00:36] <thumper> hmm
[00:39]  * thumper pokes supercommand internals some mroe
[00:43]  * thumper gets hit by python vs. go differences again
[00:48] <thumper> hmm...
[00:48] <thumper> davecheney: how clever is go in matching similar names
[00:48] <thumper> eg. I have: type A struct {val Foo}
[00:48] <thumper> and a method
[00:48] <thumper> func bar() {
[00:49] <thumper> no...
[00:49] <thumper> func bar(val Foo) {
[00:49] <thumper> a = A{val: val}
[00:49] <thumper> will it match those properly?
[00:49] <thumper> or do I have to rename something
[00:55] <davecheney> thumper: o_O
[00:55] <thumper> davecheney: this is relatively normal python and c++
[00:56] <davecheney> didn't undestand your example
[00:56] <davecheney> say again, over
[00:56] <thumper> davecheney: I have a function parameter that matches the name inside a structure
[00:56] <thumper> that I'm trying to initialise
[00:56] <thumper> so I'm trying to create an A
[00:56] <thumper> where A.val == parameter val
[00:56] <davecheney> yup, no problem there, they are different names
[00:57] <thumper> so A{val: val} is fine?
[00:57] <davecheney> sure
[00:57] <thumper> coolio
[00:57] <davecheney> val: isn't a symbol
[00:57] <davecheney> it's the name of a struct member
[00:57]  * thumper nods
[00:57] <davecheney> s/symbol/identifier
[00:57] <davecheney> when embedding you always have to disambiguate
[00:57] <davecheney> ie
[00:58] <davecheney> type S struct { io.Reader }
[00:58] <thumper> does go have a set type?
[00:58]  * thumper wants an ordered set
[00:58] <davecheney> actually, that is a crap example
[00:58] <davecheney> no, no set type
[00:58] <davecheney> you can use a map
[00:58] <thumper> :(
[00:58] <davecheney> maps are not ordered
[00:58] <davecheney> map iteration is random
[00:58] <thumper> no, but I want one
[00:58] <thumper> plz add to go 1.1
[00:59] <thumper> set + ordered_set
[00:59] <thumper> and ordered_map
[00:59] <thumper> :)
[00:59] <davecheney> the standard way is to extract the keys of a map
[00:59] <davecheney> sort them
[00:59] <davecheney> then using that sorted slice
[00:59] <davecheney> you get the picture
[00:59]  * thumper loves python
[00:59] <thumper> for key, val in sorted(some_dict):
[00:59] <davecheney> it is what it is
[01:00] <thumper> hmm.. missed the .items()
[01:00]  * thumper nods
[01:00]  * thumper will work around it
[01:02] <thumper> davecheney: so... if a second parameter is passed to make([]string, size)
[01:03] <thumper> davecheney: is the underlying array that size?
[01:03] <thumper> so meaning make([]string, 4) -> {"","","",""} ?
[01:03] <thumper> or is it actually empty with just enough space for 4 elements?
[01:04]  * thumper feels like he needs to read the language spec again
[01:05] <thumper> hmm. found it
[01:05] <thumper> make([]T, length)
[01:05] <thumper> make([]T, length, capacity)
[01:05] <thumper> hmm
[01:05] <thumper> is foo := []string
[01:05] <thumper> an array or a slice?
[01:06] <thumper> actually: foo := []string{}
[01:06] <davecheney> []slice
[01:07] <davecheney> [1234]array
[01:07] <davecheney> var foo []string -> nil slice, len and cap are 0
[01:07] <davecheney> foo := []string{} -> non nil slice, len and cap are 0
[01:08] <davecheney> foo := make([]string, 0) -> non nil slice, len and cap are 0
[01:08] <davecheney> if you are using reflect there is a difference between the 1st, and second and third arguments
[01:08] <davecheney> otherwise there is no difference
[01:09] <thumper> so... what I want is foo := make([]string, 0, len(otherthing))
[01:09] <thumper> as I don't want anything initially in foo
[01:09] <thumper> but I want to avoid unnecessary allocations
[01:10] <thumper> and I know at max there will be len(otherthing) elements
[01:10] <thumper> sound reasonable?
[01:10] <davecheney> sure
[01:10] <thumper> kk
[01:10] <davecheney> most times we don't bother setting a capacity
[01:10] <davecheney> just
[01:10] <davecheney> var foo []string ; for _, v := range thing { foo = append(foo, v) }
[01:11] <davecheney> ^ as an example
[01:11] <thumper> why?
[01:11] <thumper> how often would the underlying array resize?
[01:11] <thumper> it just seems like good manners to me
[01:12] <thumper> perhaps I'm over thinking
[01:15] <davecheney> i forget the exact mthod
[01:15] <davecheney> doubling up to 1000 elements then +1000 I think
[01:15] <davecheney> i can find out
[01:17] <thumper> so, adding 20 items causes how many allocations to an empty slice?
[01:17] <thumper> with a default capacity of zero
[01:17] <thumper> it means initially you are getting quite a few, unless the 0 -> next is something like 10
[01:18] <thumper> the default capacity specified for C++ std::vectors is 10 (by most implementations) for this exact reason
[01:18] <thumper> hmm...
[01:18] <thumper> not doubling all the time was a cause of a bug I hit with solaris c++ std::stringstreams a long while back
[01:18] <thumper> it was iterative
[01:19] <thumper> and not expecting the amount of data I was throwing at it
[01:19] <davecheney> I can check, i think it might start at 8
[01:19] <davecheney> but I am guessing
[01:19] <davecheney> if you think it is important, then set the capacity when known
[01:19] <davecheney> but avoid getting distracted by all the other places in the code where we do not do so
[01:19] <thumper> I only think it is important because I want to avoid unnecessary allocations
[01:20] <thumper> however
[01:20] <thumper> there will likely be absolutely zero notice by a user
[01:20] <thumper> so I'll keep the code simple
[01:34]  * thumper on the move again
[02:55] <thumper> bollocks, bollocks, bollocks
[02:55]  * thumper hacks and slashes recent changes
[03:31] <thumper> davecheney: back?
[03:31] <thumper> if I have a type of []string, and a method that takes (args ...string)
[03:31] <thumper> how to I pass the former into the latter?
[03:34] <thumper> found it
[03:37] <davecheney> yup, varags syntax
[03:37] <davecheney> func F(args ...string)
[03:37] <davecheney> var s []string
[03:37] <davecheney> F(s...)
[03:43] <thumper> davecheney: found it in the spec
[03:43] <thumper> quite a handy doc that
[03:43] <thumper> I'm liking how fast go compiles
[03:43] <davecheney> yeah, that is nice
[03:46] <thumper> davecheney: am I right in thinking that len(nil) == 0?
[03:47] <davecheney> thumper: depends on the type of nil
[03:48] <davecheney> that is capital T type
[03:48] <davecheney> i'm not really sure
[03:48] <davecheney> len of a nil slice or map is 0
[03:48] <thumper> ok, it is for a nil slice
[03:49] <davecheney> in that case, yes, it len of a nil slice of map is 0
[03:49] <davecheney> the of map is new in 1.1
[03:49] <davecheney> no, sorry
[03:49] <davecheney> it in 1.0
[03:49] <davecheney> there is another simplification for map in 1.1
[04:02] <thumper> davecheney: I have another branch, this one changes the way aliases for supercommands are handed
[04:02] <thumper> handled
[04:02] <davecheney> nice
[04:03] <thumper> just running all the tests now prior to proposing
[04:03] <davecheney> propose!!
[04:03] <davecheney> screw that, proposing takes minutes
[04:03] <thumper> now we get aliases shown in the generated help
[04:03] <davecheney> tests will be done by then
[04:03] <thumper> :)
[04:03] <thumper> it takes about 90s to run all the tests here
[04:03] <thumper> actually, I should time it again
[04:04] <thumper> I found myself after lunch futsing around too much with the aliases in the supercommand
[04:04] <davecheney> the work stealing algo for running multiple tests packages could be better
[04:04] <thumper> in order to provide the right help
[04:04] <thumper> ended up reverting it all
[04:04] <davecheney> but butter still would not be taking 90 seconds for a test
[04:04] <thumper> then changing the way it was done in a different branch
[04:05] <thumper> if 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 aliases
[04:06] <thumper> hmm lbox died on a launchpad api error
[04:08] <davecheney> yeah, it does that
[04:08] <davecheney> just do it again
[04:08] <davecheney> i've learnt to keep my commit messages
[04:08] <davecheney> in a text file
[04:12] <thumper> lbox is proposing...
[04:13] <thumper> https://code.launchpad.net/~thumper/juju-core/command-aliases/+merge/149461
[04:13] <thumper> so... I'm on dinner tonight
[04:13]  * thumper thinks
[04:13] <thumper> I think we decided to go out for my daughter's birthday
[04:13] <thumper> that makes it easy
[04:14] <thumper> https://codereview.appspot.com/7370043
[04:17] <davecheney> have a good one
[04:29] <thumper> I will
[04:29] <thumper> it is at my favourite resturant here
[04:29] <thumper> soooo... good
[04:31] <davecheney> how fortuitus
[08:20] <rogpeppe> morning all
[08:20] <rogpeppe> evening davecheney :-)
[08:20] <dimitern> rogpeppe: morning
[08:24] <davecheney> evening all
[08:24] <dimitern> davecheney: hiya
[08:26] <davecheney> rogpeppe: do you think we have the same set of issues that we had with bootstrapping precise from quantal
[08:26] <davecheney> that we do for ppl wanting to do 386 -> amd64, and all the other permutations
[08:26] <rogpeppe> davecheney: in this case, we just want to use 386 all down the line
[08:27] <rogpeppe> davecheney: so i don't think the same problems arise
[08:27] <rogpeppe> davecheney: although there may well be things i don't know about
[08:28] <davecheney> rogpeppe: i think there are wrinkles
[08:28] <davecheney> for example
[08:28] <davecheney> _most_ ec2 instances are amd64
[08:28] <davecheney> except for standard (?) and micro
[08:29] <davecheney> so i think it is more compliated than saying 'i want this environment to have amd64 quantal machines'
[08:29] <fwereade> davecheney, rogpeppe: this question is uppermost in my mind
[08:29] <fwereade> davecheney, rogpeppe: and I *think* it's ok
[08:29] <davecheney> fwereade: my initial response is to say 'juju only makes amd64 machines'
[08:29] <davecheney> and ignore the arch on the host machine
[08:30] <fwereade> davecheney, certainly we must ignore the arch on the host
[08:30] <davecheney> so bootstraping from anything, osx runing on arm produces amd64 machines
[08:30] <davecheney> fwereade: then that might solve rogpeppe 's question
[08:30] <fwereade> davecheney, I think that the tighest constraint we have is what arches we have the tools built for
[08:30] <davecheney> fwereade: i'd be relieved to say, for the future that we can predict, amd64 is all we do
[08:30] <rogpeppe> davecheney: what about --upload-tools ?
[08:31] <davecheney> rogpeppe: fuk
[08:31] <davecheney> double shit
[08:31] <rogpeppe> davecheney: that's my motivating case at the moment
[08:31] <rogpeppe> fwereade: mornin' BTW. hope you're feeling a little better.
[08:31] <fwereade> davecheney, I don't think there's a compelling reason to drop i386 support on ec2
[08:32] <fwereade> rogpeppe, heyhey, pretty crappy tbh, I will be off and on
[08:32] <davecheney> fwereade: apart from it being a wrinkle we don't need
[08:32] <rogpeppe> davecheney: i'm not sure i see the problem.
[08:32] <rogpeppe> davecheney: currently the logic chooses the image based on the arch and series of the tools found.
[08:32] <davecheney> rogpeppe: it's just more perumutations that we don't need righ tnow
[08:33] <rogpeppe> davecheney: i *think* all we need to do is provide a mongo db binary and it'll just work
[08:33] <davecheney> rogpeppe: i don't think we need to do that
[08:33] <davecheney> we should always request an amd64 image
[08:33] <davecheney> if it happens that the development tools are 386
[08:34] <rogpeppe> davecheney: not if the caller wants to upload tools
[08:34] <davecheney> that isn't a problem on linux
[08:34] <rogpeppe> davecheney: and have them run on the bootstrap machine
[08:34] <davecheney> yeah, that works fine
[08:34] <davecheney> just clamp the AMI request to amd64, job done
[08:35] <rogpeppe> davecheney: ah, because you can always run 386 binaries ok
[08:35]  * dimitern rebooting
[08:35] <davecheney> yeah
[08:35] <davecheney> going the other way, amd64 tools on a t1.micro will be sadface
[08:35] <davecheney> but that is a bridge to cross at another time
[08:35] <fwereade> davecheney, is that any worse than *anything* on a t1.micro? ;p
[08:36] <davecheney> exactly
[08:36] <davecheney> it's not the way the market is heading
[08:36] <rogpeppe> davecheney: 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] <davecheney> rogpeppe: --upload-tools is the hack
[08:36] <davecheney> i'm fine with hackig it some more
[08:36] <davecheney> rogpeppe: is there an issue for this work ?
[08:37] <rogpeppe> davecheney: yeah, wait a mo
[08:37] <rogpeppe> davecheney: https://bugs.launchpad.net/juju-core/+bug/1130209
[08:37] <_mup_> Bug #1130209: we need a mongodb compiled for i386 <juju-core:New> < https://launchpad.net/bugs/1130209 >
[08:38] <rogpeppe> davecheney: but that's fairly specific - i assumed it was the right solution
[08:38] <davecheney> rogpeppe: mind if I rename that issue to what I see is the problem ?
[08:39] <rogpeppe> davecheney: ok
[08:39] <rogpeppe> davecheney: which is?
[08:41] <davecheney> "juju should always request an amd64 AMI image"
[08:41] <davecheney> actually, that will probably mean we can't boot t1.micros
[08:41] <davecheney> but whatever
[08:42] <fwereade> davecheney, I *think* that we can start any ec2 instance with AMD64
[08:42] <fwereade> davecheney, but I'm not quite convinced this is an ok shortcut
[08:43] <davecheney> fwereade: why not ?
[08:43] <fwereade> davecheney, haven't people already used juju on ARM machines? even if they don't run the state server?
[08:43] <rogpeppe> fwereade: i've made some replies to https://codereview.appspot.com/7326052/
[08:44] <davecheney> fwereade: only as a client
[08:44] <davecheney> my view of the crystal ball shows only environments containing amd64 machines
[08:44] <davecheney> when highbank or calexeda come asking, we fix the problem then
[08:44] <fwereade> davecheney, what about the arm stuff that had to be added to python juju for maas?
[08:45] <rogpeppe> fwereade: 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] <davecheney> fwereade: in the spirit of delivering 13.04, we have to leave the goal posts in the ground
[08:45] <fwereade> davecheney, I do not believe I am the person trying to move them...
[08:45] <rogpeppe> davecheney: what *is* the problem with just uploading 386 mongo binaries, BTW?
[08:46] <davecheney> rogpeppe: well, 1, we don't have those binaries, so that is work
[08:46] <rogpeppe> davecheney: i pointed you at some
[08:46] <davecheney> 2, getting the deps in the archive is going to be an amazing ballache, so i'd rather no x2 it
[08:46] <rogpeppe> davecheney: i don't understand - which deps in which archive?
[08:47] <davecheney> rogpeppe: with respect, I have proposed a solution that has a low cost now, and a known cost in the future
[08:47] <davecheney> we don't have to gold plate this
[08:47] <davecheney> we just have to make it work on jam's dev machine, right ?
[08:47] <rogpeppe> davecheney: if i'd had the credentials, i could have fixed this with a single s3 upload last night
[08:48] <davecheney> rogpeppe: how can I send them too you ?
[08:48] <davecheney> i'm more than happy for someone else to fix this
[08:48] <rogpeppe> davecheney: i have the ones you gave me before, but they don't seem to work any more
[08:49] <davecheney> yeah, niemeyer reset them a few times trying to get the permission junk working
[08:49] <rogpeppe> davecheney: ah
[08:49] <davecheney> rogpeppe: email me your public key
[08:49] <davecheney> i'll bootstrap a machine
[08:49] <davecheney> and leave them there for you
[08:51] <fwereade> rogpeppe, 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] <rogpeppe> fwereade: well, surely the *place* is right - it's just the code is wrong?
[08:52] <fwereade> rogpeppe, the code cannot be correct while it's in that place, though, can it?
[08:52] <rogpeppe> fwereade: the code cannot be correct in any place!
[08:53] <rogpeppe> fwereade: 'cos it's buggy
[08:53] <fwereade> rogpeppe, yeah, but the bugs are intractable while it's outside the state package
[08:53] <rogpeppe> fwereade: do you think we should just delete set and get support because they're currently wrong?
[08:54] <fwereade> rogpeppe, I think that picking something that's wrong as a demonstration of how to do something right is problematic
[08:54] <rogpeppe> fwereade: 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:58] <fwereade> rogpeppe, 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:59] <fwereade> rogpeppe, 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 layer
[09:00] <rogpeppe> fwereade: the benefit of the extra layer is at least we're not duplicating the buggy code
[09:00] <rogpeppe> fwereade: because it needs to be called from two places now
[09:01] <fwereade> rogpeppe, er, what? why would we be duplicating it if we put it in one place, ie in state?
[09:01] <fwereade> rogpeppe, both those places use the state API though, right?
[09:03] <rogpeppe> fwereade: 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] <rogpeppe> fwereade: i thought about putting them in state too
[09:04] <rogpeppe> fwereade: it depends whether we feel that almost all the code in juju/cmd should actually be done in state.
[09:04] <rogpeppe> cmd/juju
[09:06] <fwereade> rogpeppe, well that case obviously has to be; and offhand I can't think of one that shouldn't, can you?
[09:07] <rogpeppe> fwereade: so what should i say to the GUI folks? hold off while we refactor cmd/juju ?
[09:08] <fwereade> rogpeppe, I would say "put it in state, like this"... maybe I'm not understanding some of the constraints?
[09:09] <fwereade> rogpeppe, blast, sorry, need a quick break, can we pick this up a bit later?
[09:09] <rogpeppe> fwereade: ok
[09:40] <fwereade> rogpeppe, hey, sorry to abandon you; I think I might be returning to bed for a while again soon, but we should finish this
[09:40] <rogpeppe> fwereade: shall we G+?
[09:40] <fwereade> rogpeppe, sgtm, will you start?
[09:40] <rogpeppe> fwereade: ok
[09:43] <rogpeppe> fwereade: ha, i've made a hangout, but the window has disappeared
[09:43]  * fwereade tries
[11:00] <fwereade> jam, ping: https://codereview.appspot.com/7307128/patch/1/6
[11:01] <fwereade> jam, for your first comment, I thought it was covered by "// Environ constraints are not available before initialization." just above?
[11:01] <fwereade> jam, and for the second: yes to the first bit, clarify please to the seconds
[11:02] <fwereade> jam, I'm still pretty flaky today so am appearing and disappearing at random, just answer whenever and I'll see it when I see it
[11:32] <dimitern> jam, mgz: standp
[12:14] <rogpeppe> fwereade: 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:29] <fwereade> rogpeppe, am I right in thinking there's no api.IsNotFound?
[12:32] <fwereade> rogpeppe, also, who should I be talking to to figure out the deal with yaml-in-the-gui?
[12:33] <fwereade> rogpeppe, because I'm still pretty strongly -1 on the two-parallel-paths-not-checked-on-the server bit
[12:49] <fwereade> dimitern, 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 LGTM
[12:50] <rogpeppe> fwereade: hmm, sorry missed your remarks above
[12:50] <rogpeppe> fwereade: from the top:
[12:51] <rogpeppe> fwereade: yes, there's no api.IsNotFound - it's spelt api.ErrCode() == api.CodeNotFound instead
[12:52] <fwereade> rogpeppe, how about something more like api.ErrHasCode(err, api.CodeNotFound), perhaps?
[12:53] <fwereade> rogpeppe, just bikeshedding really, but I thought that IsNotFound was pretty handy
[12:53] <rogpeppe> fwereade: in #juju-gui, bac or gary_poster are two possibilities
[12:53] <fwereade> rogpeppe, cool, ty
[12:53] <rogpeppe> fwereade: in ec2, i started with HasCode, but it turns out it's nicer to have a function that returns the error code.
[12:54] <fwereade> rogpeppe, ok, fair enough on that, not sure I really like my suggestion
[12:54] <rogpeppe> fwereade: for one thing, in tests, c.Assert(api.ErrCode(err), Equals, api.CodeNotFound) gives a nicer error message on failure
[12:54] <rogpeppe> fwereade: also, you can do switch code := api.ErrCode(err); code {
[12:55] <fwereade> rogpeppe, ok, sgtm, thanks :)
[12:55] <rogpeppe> fwereade: np
[12:55] <rogpeppe> fwereade: FWIW i don't think the two parallel paths thing is really a problem - i just documented that Config overrides Options.
[12:56] <rogpeppe> fwereade: but there's an issue around that that i was discussing with niemeyer yesterday (in your absence)
[12:56] <rogpeppe> fwereade: 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:57] <fwereade> rogpeppe, 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 params
[12:57] <fwereade> rogpeppe, isn't an empty string "fall back to the default" in this context?
[12:57] <rogpeppe> fwereade: if only js had decent yaml support... (if only yaml wasn't insane...)
[12:57] <fwereade> rogpeppe, ha, yeah
[12:58] <rogpeppe> fwereade: yes, but it could also be "set this to the empty string"
[12:58] <rogpeppe> fwereade: and in yaml you can express that difference
[12:58] <rogpeppe> fwereade: my inclination is to always make an empty string equivalent to null
[12:58] <rogpeppe> fwereade: then the two paths are equivalent
[12:59] <fwereade> rogpeppe, yeah, I think we kinda decided that a while ago, but that might just be in informal inference I made
[12:59] <rogpeppe> fwereade: FWIW niemeyer thought that it was reasonable to have yaml be more expressive.
[13:00] <TheMue> fwereade: You've got a +1.
[13:00] <rogpeppe> fwereade: BTW if it wasn't for the js stuff, i would have no hesitation at all about dropping the yaml path
[13:00] <TheMue> fwereade: And thx for your review, the change is proposed again.
[13:00] <fwereade> rogpeppe, 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 yaml
[13:00] <fwereade> TheMue, tyvm, I'll take a look in a bit
[13:05] <rogpeppe> fwereade: i just had a little chat in #juju-gui - here's the result: http://paste.ubuntu.com/1690301/
[13:07] <rogpeppe> fwereade: 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:08] <rogpeppe> fwereade: alternatively, we could have two entry points: ServiceSet and ServiceSetYAML
[13:08] <rogpeppe> fwereade: under the surface they'd invoke the same thing
[13:09] <fwereade> rogpeppe, I think I'd rather separate them like that if nothing else
[13:09] <rogpeppe> fwereade: ok, although i still don't really understand the problem
[13:10] <fwereade> rogpeppe, or... they could just JSON it in the GUI, and that's still valid YAML, right?
[13:10] <fwereade> rogpeppe, so long as we handle ""/null consistently
[13:10] <rogpeppe> fwereade: true. i suppose my ickiness head goes "json-in-json-string, yeuch"
[13:11] <fwereade> rogpeppe, I think that with the YAML requirement we're already way over that line
[13:12] <rogpeppe> fwereade: that's only required if you want to upload yaml. i'd prefer not to pollute the clean path because of that requirement
[13:14] <fwereade> rogpeppe, can you live with two separate calls?
[13:16] <fwereade> rogpeppe, that seems to me like the least polluting approach possible ;)
[13:17] <rogpeppe> fwereade: yeah, i'll do two separate calls.
[13:25] <fwereade> rogpeppe, cool, tyvm
[13:26] <fwereade> TheMue, I'm wondering about the 301s on paths with ..
[13:26] <fwereade> TheMue, surely if anything's a 403, it's attempts to read/write outside the path?
[13:27] <fwereade> TheMue, and a prefix of "../" acting the same as no prefix is a bit weird as well I think
[13:27] <fwereade> TheMue, when listing I mean
[13:29] <TheMue> fwereade: I've wondered too and it took me some time to discover it.
[13:29] <TheMue> fwereade: It's a feature of the http daemon of Go.
[13:29] <TheMue> fwereade: It resolves relative paths and sends a redirect to the client.
[13:30] <TheMue> fwereade: And I also wondered about why using it with a pattern doesn't lead to a 301. Strange.
[13:32] <fwereade> TheMue, hmm, where does it redirect to?
[13:33] <TheMue> fwereade: I have to check now that the URL doesn't contain the environ name anymore.
[13:34] <TheMue> fwereade: Before it has been from /environname/../foo to /foo
[13:34] <TheMue> fwereade: But only a redirect, I client has to interpret it itself.
[13:35] <fwereade> TheMue, hmm, would you look into that please? it should not have been doing that, that gets access to the files in the foo environment
[13:36] <TheMue> fwereade: 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] <TheMue> fwereade: Right now we don't do any authentication on the storage.
[13:36] <fwereade> TheMue, so where does localhost:60006/../foo redirect to?
[13:38] <TheMue> fwereade: 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] <TheMue> fwereade: If that does then react on it and does a request and the file doesn't exists it gets a 404.
[13:39] <fwereade> TheMue, 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 moot
[13:39] <TheMue> fwereade: But give me some minutes, now I'm interested in that redirect too.
[13:39] <fwereade> TheMue, but nonetheless I would rather have a PUT to ../wherever/blah *fail* rather than just put *somewhere* ;)
[13:39] <fwereade> TheMue, thanks
[13:40] <TheMue> fwereade: Sorry, the Go server works differently. I have to look if it provides a kind of hook to get involved in the resolving.
[13:41] <TheMue> fwereade: And security has nothing to do with the generated redirect, here we should introduce a real authentication feature.
[13:41] <TheMue> fwereade: At least basic auth.
[13:43] <fwereade> TheMue, what scenarios are we protecting against here?
[13:44] <TheMue> fwereade: One moment, testing.
[13:45] <fwereade> TheMue, 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:46] <fwereade> TheMue, 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 auth
[13:46] <TheMue> fwereade: GET /../* is directly delivered (no redirect, as I said) as /*
[13:47] <fwereade> TheMue, ok, that's fair enough I suppose; but GET /../foo redirects?
[13:48] <TheMue> fwereade: 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:49] <fwereade> TheMue, it *is* a dev tool, not a production system... and, hold on, can we not restrict it to connections from localhost?
[13:49] <TheMue> fwereade: /../foo* to /foo*
[13:50] <fwereade> TheMue, so that one's a redirect?
[13:50] <TheMue> fwereade: 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:51] <fwereade> TheMue, ok, sorry: GET never redirects, but PUT and DELETE do?
[13:51] <TheMue> fwereade: Restriction by IP should be possible, but not localhost, but the IP of the LXC containers.
[13:52] <fwereade> TheMue, it's not clear that we ever want something in a container to use it as a Storage
[13:52] <TheMue> fwereade: Hehe, ok, step by step.
[13:52] <mgz> right, I was wondering about access control on the local provider storage too
[13:52] <rogpeppe> fwereade: 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] <fwereade> TheMue, when should a container be able to get a Storage to work with?
[13:53] <TheMue> fwereade, mgz: Please seperate those discussions, first redirect, later security.
[13:53] <TheMue> fwereade: Once again, step by step.
[13:53] <mgz> TheMue: I'll try to leave comments on the review
[13:53] <rogpeppe> mgz: eventually, we could probably use admin-secret for security, if we cared much.
[13:54] <mgz> rogpeppe: it's probably less security and more accidental clashing you care about for local
[13:55] <TheMue> fwereade: First approach has been to use a URL like host:1234/<<environ>>/<<name>>.
[13:55] <TheMue> fwereade: In that case a ".." leads to a redirect.
[13:55] <rogpeppe> mgz: 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] <TheMue> fwereade: Now we use host:1234/<<name>>.
[13:56] <TheMue> fwereade: Here /../<<name>> doesn't lead to a redirect, but the ".." vanishes.
[13:56] <fwereade> TheMue, ok, with all verbs?
[13:56] <TheMue> fwereade: But in case of a PUT, the Go HTTP daemon returns a 301.
[13:56] <fwereade> TheMue, ah, ok, sorry
[13:57] <TheMue> fwereade: Needed some time to get behind it as it screwed up my expectations and tests. :D
[13:57] <fwereade> TheMue, ...and *that* redirects to..?
[13:57] <fwereade> TheMue, /<<name>> ?
[14:00] <TheRealMue> fwereade: But again, it doesn't deliver ata, it does no existence check, nothing.
[14:02] <fwereade> TheRealMue, 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] <TheRealMue> fwereade: Yeah, PUT /../foo redirects to /foo.
[14:02] <TheRealMue> fwereade: I'll document it, yip.
[14:02] <fwereade> TheRealMue, tyvm
[14:03] <TheRealMue> fwereade: It's simply a "if contains("..") { transformAndRedirect(url) }. ;)
[14:08]  * rogpeppe goes to lunch
[14:24] <niemeyer> Yo
[14:31] <dimitern> niemeyer: hey
[14:39] <fwereade> niemeyer, heyhey
[14:57] <fwereade> rogpeppe, mramm, TheMue: kanban?
[14:57] <rogpeppe> fwereade: in 2 mins, no?
[14:57] <TheMue> fwereade: Yep, in 2 mins.
[14:58] <fwereade> rogpeppe, what's the deal with 208-simplify-config-tests? did that go anywhere, or was another variant chosen, or what?
[14:58] <rogpeppe> fwereade: ah, i was reminded of that this morning. i haven't yet addressed jtv's final comment on that CL
[15:17] <bac> rogpeppe: 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:30] <fwereade> rogpeppe, I would favour longer but more painfully explicit names, if possible
[15:31] <fwereade> rogpeppe, sorry, my comments on the second don't apply so well in the light of the decusuins re the first
[15:31] <rogpeppe> fwereade: i thought ServiceSet was ok
[15:31] <rogpeppe> fwereade: (given that we've got "juju set")
[15:31] <rogpeppe> fwereade: but you'd prefer ServiceSetConfigParams?
[15:32] <fwereade> rogpeppe, ah, hmm, I think consistency there is actually quite good
[15:32] <rogpeppe> fwereade: well, sure, but i'd have ServiceSetConfig to match
[15:32] <rogpeppe> fwereade: i just thought that was on the verge of unnecessarily verbose
[15:33] <fwereade> rogpeppe, yeah, indeed, I am newly taken with the benefits of ServiceSetParams/EnvironSetParams(?)
[15:33] <rogpeppe> fwereade: great, thanks
[15:33] <rogpeppe> bac: i changed the names as suggested in the review
[15:34] <rogpeppe> bac: the second branch will change to match - i still need to do some work on it as per discussions before landing it
[15:35] <bac> rogpeppe: ok, so the names in the landed branch are preferred.  thanks.
[15:39]  * TheMue has a late lunch today, biab
[15:48] <fwereade> rogpeppe, btw, is there a bug in LP for that `juju-log "--lol screw you"` issue?
[15:48] <rogpeppe> fwereade: no, gimme 30s
[15:49] <fwereade> rogpeppe, would you assign it to tim please, and suggest the SetFlags tweak we discussed?
[15:49] <rogpeppe> fwereade: k
[15:56] <rogpeppe> fwereade: hmm, do you know where the implementation of the jujuc commands is in the python?
[15:57] <rogpeppe> fwereade: oh, found it
[15:57] <fwereade> rogpeppe, sorry I missed you
[15:58] <rogpeppe> fwereade: 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-log
[15:58] <rogpeppe> fwereade: i can't see how that works in the python
[15:59] <rogpeppe> fwereade: (i'm looking in juju/hooks/commands.py, BTW)
[15:59] <fwereade> rogpeppe, I would guess that the parser just ignores the stuff it doesn't recognise and passes it on in the args
[15:59] <fwereade> rogpeppe, what does it use: optparse?
[16:00] <rogpeppe> fwereade: argparse
[16:01] <fwereade> rogpeppe, yeah, I think that has parse_known_args?
[16:02] <rogpeppe> fwereade: it doesn't use that method
[16:02] <rogpeppe> fwereade: it just uses parse_args
[16:03]  * rogpeppe manages to see through *some* of the python magic
[16:03] <fwereade> rogpeppe, ah, wait, it gets a bunch of stuff from juju/hooks/cli.py as well I think
[16:04] <rogpeppe> fwereade: yeah, that's the place that calls parse_args
[16:04] <rogpeppe> fwereade: in the __call__ magic method
[16:04] <mramm> anybody have a link to tim's gap analysis doc?  I can't seem to find it in my google drive.
[16:05] <rogpeppe> fwereade: hmm, i think this might be magic too:         self.parser.add_argument("message", nargs="+")
[16:05] <rogpeppe> fwereade: perhaps that "+" signifies "anything remaining"
[16:05] <fwereade> rogpeppe, very probably
[16:06] <rogpeppe> fwereade: 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 level
[16:06] <rogpeppe> fwereade: if that's true, it's really not very nice and breaks all our arg parsing rules.
[16:12] <fwereade> rogpeppe, sorry, my dad just arrived from england, was saying hello
[16:12] <rogpeppe> fwereade: np
[16:12] <fwereade> rogpeppe, I agree that that is not very nice
[16:12] <rogpeppe> fwereade: am just trying to understand argparse
[16:12] <rogpeppe> fwereade: "+" just says any remaining args - it still seems to give an error when a flag isn't recognised
[16:13] <fwereade> rogpeppe, hmm, maybe it has special handling for "--" followed by "something weird"
[16:13] <rogpeppe> fwereade: maybe - i'll try that next
[16:15] <rogpeppe> fwereade: yup, that's right. yulk
[16:16] <rogpeppe> fwereade: here's my testbed: http://paste.ubuntu.com/1691047/
[16:16] <rogpeppe> fwereade: sample: http://paste.ubuntu.com/1691052/
[16:17] <rogpeppe> fwereade: looks like if you've got a space in the arg it's treated as if it can't be a flag
[16:17] <fwereade> rogpeppe, ok, I guess that makes sense, but still, eww :)
[16:18] <rogpeppe> fwereade: no, that's not true either
[16:18] <rogpeppe> fwereade: if there's a space in the arg, it's treated as a flag *only* if the prefix doesn't match a flag
[16:19] <fwereade> rogpeppe, wait what? I don;t think that fits with what you showed me
[16:19] <fwereade> rogpeppe, treated as an arg?
[16:19] <rogpeppe> fwereade: so "tst.py '-z arg'" goes through ok but "tst.py -z ' arg' fails
[16:20] <rogpeppe> fwereade: which seems so so wrong to me
[16:20] <fwereade> rogpeppe, I dunno, it actually seems pretty sane... but it's annoying to have to replicate all the magic
[16:21] <fwereade> rogpeppe, 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] <rogpeppe> fwereade: it seems wrong to me because in general i'd think that -z$arg and -z "$arg" should be handled exactly the same
[16:21] <fwereade> rogpeppe, yeah, reasonable perspective
[16:21] <rogpeppe> fwereade: what if the create arg as "-lINFO" ?
[16:21] <rogpeppe> s/create/crazy/ s/as/was/
[16:22] <fwereade> rogpeppe, well it *is* meaningless anyway because we dropped all the logging levels :/
[16:22] <rogpeppe> fwereade: ha
[16:22] <fwereade> rogpeppe, gtg for a bit, back later
[16:22] <rogpeppe> fwereade: ok
[16:48] <rogpeppe> fwereade: https://bugs.launchpad.net/juju-core/+bug/1130771
[16:48] <_mup_> Bug #1130771: juju-log is not compatible with python version <juju-core:New> < https://launchpad.net/bugs/1130771 >
[17:10] <fwereade> rogpeppe, responded
[17:11] <TheMue> niemeyer: Your ubuntufinder.com is quite cool.
[17:11] <rogpeppe> fwereade: i wonder if the argparse semantics are actually documented anywhere
[17:14] <dimitern> niemeyer: yeah, pretty cool UI with these rolling radio buttons / shelves :)
[17:17] <rogpeppe> fwereade: 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:18] <fwereade> rogpeppe, hmm, that looks like it ought to work
[17:18] <rogpeppe> fwereade: unfortunately the provisioner never actually shuts down the instance. i'm wondering if i should remove the machine too.
[17:19] <fwereade> rogpeppe, no, that's the provisioner's job... does it really not do it? I could have sworn it did
[17:19] <rogpeppe> fwereade: yeah, i thought so too
[17:19] <fwereade> rogpeppe, if not then, well, this is why it was bad to invent our own shutdown process in those tests ;p
[17:19] <rogpeppe> fwereade: BTW, i *think* that Machine.Destroy should not require a Refresh if the last view of the machine had assigned units.
[17:20] <rogpeppe> fwereade: agreed
[17:20] <fwereade> rogpeppe, we should try regardless of the latest known state? not convinced, but go on
[17:21] <rogpeppe> fwereade: yeah, because it's what's actually out there in the state that's important AFAICS
[17:21] <rogpeppe> fwereade: in the above loop, there's no real reason we need to keep on refreshing while trying Destroy periodically
[17:22] <fwereade> rogpeppe, 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 do
[17:22] <rogpeppe> fwereade: in this case, there's nothing to watch
[17:23] <rogpeppe> fwereade: or... maybe there is
[17:23] <rogpeppe> fwereade: well, anyway, the in-memory state has no bearing on whether we can or can't call Destroy
[17:23] <fwereade> rogpeppe, a machine watch will observe those changes
[17:23] <rogpeppe> fwereade: yeah
[17:25] <fwereade> rogpeppe, 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 state
[17:25] <rogpeppe> fwereade: why should we? the in-memory state doesn't tell us anything about whether the machine has units *now*, no?
[17:26] <mgz> bah, why did I type images, I meant instance-types
[17:26] <mgz> terminology is hard
[17:26] <rogpeppe> fwereade: hmm, except, i guess, when there are *no* units and the machine is dying
[17:26] <fwereade> rogpeppe, no, but if you're interested we'll tell you when it changes... why accommodate impatient hammering of the API
[17:27] <rogpeppe> fwereade: i'm not sure why {wait for change; refresh; destroy} is better than {wait for change; destroy}
[17:27] <rogpeppe> fwereade: it certainly surprised me when i discovered it was necessary.
[17:28] <fwereade> rogpeppe, because if you didn't refresh then you obviously have no basis for assuming it's destroyable ;p
[17:28] <rogpeppe> fwereade: i know it changed :-)
[17:28] <rogpeppe> fwereade: the overhead of calling destroy is probably not far off that of calling refresh. or... maybe it is actually much cheaper in fact.
[17:30] <fwereade> rogpeppe, define cheap... destroy will potentially lock the service document, won't it?
[17:30] <rogpeppe> fwereade: yeah, i guess so
[17:31] <fwereade> rogpeppe, ah wait that's only for a unit, doesn't apply
[17:31] <rogpeppe> fwereade: oh yes, not for a machine
[17:31] <fwereade> rogpeppe, I somehow switched to thinking about units
[17:31] <rogpeppe> fwereade: 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:32] <rogpeppe> fwereade: (even though the actual state would have allowed the operation)
[17:32] <fwereade> rogpeppe, 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 through
[17:33] <fwereade> rogpeppe, 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 it
[17:34] <rogpeppe> fwereade: 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 definitive
[17:35] <fwereade> rogpeppe, that's a known bug that we officially do not care enough about to fix by 13.04
[17:36] <fwereade> rogpeppe, if the provisioner's not actually cleaning up the machine, that is an issue of vastly greater magnitude ;)
[17:37] <rogpeppe> fwereade: true 'nuff
[17:48] <frankban> hi all, does the current juju-core trunk support default-series? I have precise in my env configuration, but nodes are created using quantal.
[17:49] <rogpeppe> fwereade: do you know about the above question?
[17:50] <niemeyer> TheMue, dimitern: Cheers!
[17:51] <rogpeppe> fwereade: first watcher in the API implemented: https://codereview.appspot.com/7390043/
[17:51] <fwereade> rogpeppe, heh, you would have been the person I asked
[17:52] <fwereade> frankban, so the charm you're using is definitely a precise one?
[17:52] <rogpeppe> fwereade: darn. i should really know :-)
[17:52] <fwereade> rogpeppe, I am up to my elbos in that stuff at the moment so the behaviour will doubtless change somewhat over the next couple of says
[17:52] <fwereade> rogpeppe, but it does *kinda* look like every machine is started with the series/arch of the machine that launches it :/
[17:53] <rogpeppe> fwereade: it's started with the series/arch of whatever tools it can find, i think
[17:53] <frankban> fwereade: yes, it's the juju gui charm (deployed from local repository). however, the bootstrap node is also quantal
[17:54] <rogpeppe> fwereade: anyway, gotta go. we're packing the car up, off to the Lakes for some snowy hills
[17:54] <fwereade> rogpeppe, enjoy
[17:54] <bac> bye rogpeppe.  you around tomorrow?
[17:54] <rogpeppe> fwereade: will do. have a great rest of week. should be in contact sporadically by email
[17:54] <rogpeppe> bac: no, sorry. away for rest of week.
[17:55] <bac> rogpeppe: have fun!
[17:55] <rogpeppe> bac: will try me best :-
[17:55] <rogpeppe> )
[17:56] <fwereade> frankban, ok, that just sucks profoundly... but, yeah, I don't see how that's meant to actually work
[17:57] <fwereade> frankban, it seems that the machine we start is to all intents and purposes random
[17:57] <fwereade> frankban, good thing I am working on that right now
[17:58] <frankban> fwereade: cool, thanks
[17:59] <rogpeppe> see y'all monday!
[21:22] <bac> hi, i've put a small branch up for review at https://codereview.appspot.com/7401043/
[21:29] <thumper> bac: looks good to me, but I have no confidence in my ability to review it yet
[21:30] <thumper> bac: particularly to know if it works or is even right :)
[21:34] <thumper> Q) an uninitialized string in a struct has a value of "" ?
[21:39] <bac> thumper: yes
[21:39] <thumper> cool...
[21:40] <bac> thumper: 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] <thumper> hopefully within a week or so I'll feel happier commenting on merge proposals
[21:42] <bac> thumper: is the requirement two reviews?
[21:42] <thumper> yeah
[22:40] <davecheney> thumper: will review your CL in two secs
[22:40] <thumper> sure
[22:46]  * thumper afk for lunch