=== 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 [00:27] niemeyer: OT, i met Benny Siegert last night [00:27] he now works for google as an SRE [00:27] and was visiting Sydney [00:28] davecheney: Benny.. hmm [00:29] davecheney: I think I had a couple of interactions with him only in golang-nuts@ [00:29] davecheney: There was an event yesterday, right? How was it? [00:30] excellent [00:30] numbers are improving every time [00:30] we had 50 people [00:30] more maybe [00:30] the room was packed [00:31] davecheney: a go meetup? [00:32] yup [00:32] that was why I couldn't make the meeting last night [00:32] I run the sydney golang meetup group [00:32] ah [00:33] I think I've just found a real use of the struct composition with methods... [00:33] the alias struct in supercommand.go [00:33] it overrides Info [00:33] but not the other methods [00:34] those get resolved to Command? [00:34] I do have to be honest in thinking that this looks like bastardised inheritance [00:34] thumper: it's much less than that [00:34] it is no inheritence at all [00:35] so how does it resolve the correct function calls? [00:35] always at compilation time [00:35] sure, how [00:35] embedded fields are just syntactic sugar [00:36] type S struct { io.Reader } [00:36] var s S [00:36] s.Read is just sugar for s.Reader.Read [00:36] ^ one example [00:36] only if s.Read doesn't exist? [00:36] yup [00:36] hmm [00:39] * thumper pokes supercommand internals some mroe [00:43] * thumper gets hit by python vs. go differences again [00:48] hmm... [00:48] davecheney: how clever is go in matching similar names [00:48] eg. I have: type A struct {val Foo} [00:48] and a method [00:48] func bar() { [00:49] no... [00:49] func bar(val Foo) { [00:49] a = A{val: val} [00:49] will it match those properly? [00:49] or do I have to rename something [00:55] thumper: o_O [00:55] davecheney: this is relatively normal python and c++ [00:56] didn't undestand your example [00:56] say again, over [00:56] davecheney: I have a function parameter that matches the name inside a structure [00:56] that I'm trying to initialise [00:56] so I'm trying to create an A [00:56] where A.val == parameter val [00:56] yup, no problem there, they are different names [00:57] so A{val: val} is fine? [00:57] sure [00:57] coolio [00:57] val: isn't a symbol [00:57] it's the name of a struct member [00:57] * thumper nods [00:57] s/symbol/identifier [00:57] when embedding you always have to disambiguate [00:57] ie [00:58] type S struct { io.Reader } [00:58] does go have a set type? [00:58] * thumper wants an ordered set [00:58] actually, that is a crap example [00:58] no, no set type [00:58] you can use a map [00:58] :( [00:58] maps are not ordered [00:58] map iteration is random [00:58] no, but I want one [00:58] plz add to go 1.1 [00:59] set + ordered_set [00:59] and ordered_map [00:59] :) [00:59] the standard way is to extract the keys of a map [00:59] sort them [00:59] then using that sorted slice [00:59] you get the picture [00:59] * thumper loves python [00:59] for key, val in sorted(some_dict): [00:59] it is what it is [01:00] hmm.. missed the .items() [01:00] * thumper nods [01:00] * thumper will work around it [01:02] davecheney: so... if a second parameter is passed to make([]string, size) [01:03] davecheney: is the underlying array that size? [01:03] so meaning make([]string, 4) -> {"","","",""} ? [01:03] 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] hmm. found it [01:05] make([]T, length) [01:05] make([]T, length, capacity) [01:05] hmm [01:05] is foo := []string [01:05] an array or a slice? [01:06] actually: foo := []string{} [01:06] []slice [01:07] [1234]array [01:07] var foo []string -> nil slice, len and cap are 0 [01:07] foo := []string{} -> non nil slice, len and cap are 0 [01:08] foo := make([]string, 0) -> non nil slice, len and cap are 0 [01:08] if you are using reflect there is a difference between the 1st, and second and third arguments [01:08] otherwise there is no difference [01:09] so... what I want is foo := make([]string, 0, len(otherthing)) [01:09] as I don't want anything initially in foo [01:09] but I want to avoid unnecessary allocations [01:10] and I know at max there will be len(otherthing) elements [01:10] sound reasonable? [01:10] sure [01:10] kk [01:10] most times we don't bother setting a capacity [01:10] just [01:10] var foo []string ; for _, v := range thing { foo = append(foo, v) } [01:11] ^ as an example [01:11] why? [01:11] how often would the underlying array resize? [01:11] it just seems like good manners to me [01:12] perhaps I'm over thinking [01:15] i forget the exact mthod [01:15] doubling up to 1000 elements then +1000 I think [01:15] i can find out [01:17] so, adding 20 items causes how many allocations to an empty slice? [01:17] with a default capacity of zero [01:17] it means initially you are getting quite a few, unless the 0 -> next is something like 10 [01:18] the default capacity specified for C++ std::vectors is 10 (by most implementations) for this exact reason [01:18] hmm... [01:18] not doubling all the time was a cause of a bug I hit with solaris c++ std::stringstreams a long while back [01:18] it was iterative [01:19] and not expecting the amount of data I was throwing at it [01:19] I can check, i think it might start at 8 [01:19] but I am guessing [01:19] if you think it is important, then set the capacity when known [01:19] but avoid getting distracted by all the other places in the code where we do not do so [01:19] I only think it is important because I want to avoid unnecessary allocations [01:20] however [01:20] there will likely be absolutely zero notice by a user [01:20] so I'll keep the code simple [01:34] * thumper on the move again [02:55] bollocks, bollocks, bollocks [02:55] * thumper hacks and slashes recent changes [03:31] davecheney: back? [03:31] if I have a type of []string, and a method that takes (args ...string) [03:31] how to I pass the former into the latter? [03:34] found it [03:37] yup, varags syntax [03:37] func F(args ...string) [03:37] var s []string [03:37] F(s...) [03:43] davecheney: found it in the spec [03:43] quite a handy doc that [03:43] I'm liking how fast go compiles [03:43] yeah, that is nice [03:46] davecheney: am I right in thinking that len(nil) == 0? === gary_poster is now known as gary_poster|away [03:47] thumper: depends on the type of nil [03:48] that is capital T type [03:48] i'm not really sure [03:48] len of a nil slice or map is 0 [03:48] ok, it is for a nil slice [03:49] in that case, yes, it len of a nil slice of map is 0 [03:49] the of map is new in 1.1 [03:49] no, sorry [03:49] it in 1.0 [03:49] there is another simplification for map in 1.1 [04:02] davecheney: I have another branch, this one changes the way aliases for supercommands are handed [04:02] handled [04:02] nice [04:03] just running all the tests now prior to proposing [04:03] propose!! [04:03] screw that, proposing takes minutes [04:03] now we get aliases shown in the generated help [04:03] tests will be done by then [04:03] :) [04:03] it takes about 90s to run all the tests here [04:03] actually, I should time it again [04:04] I found myself after lunch futsing around too much with the aliases in the supercommand [04:04] the work stealing algo for running multiple tests packages could be better [04:04] in order to provide the right help [04:04] ended up reverting it all [04:04] but butter still would not be taking 90 seconds for a test [04:04] then changing the way it was done in a different branch [04:05] 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] hmm lbox died on a launchpad api error [04:08] yeah, it does that [04:08] just do it again [04:08] i've learnt to keep my commit messages [04:08] in a text file [04:12] lbox is proposing... [04:13] https://code.launchpad.net/~thumper/juju-core/command-aliases/+merge/149461 [04:13] so... I'm on dinner tonight [04:13] * thumper thinks [04:13] I think we decided to go out for my daughter's birthday [04:13] that makes it easy [04:14] https://codereview.appspot.com/7370043 [04:17] have a good one [04:29] I will [04:29] it is at my favourite resturant here [04:29] soooo... good [04:31] how fortuitus [08:20] morning all [08:20] evening davecheney :-) [08:20] rogpeppe: morning [08:24] evening all [08:24] davecheney: hiya [08:26] rogpeppe: do you think we have the same set of issues that we had with bootstrapping precise from quantal [08:26] that we do for ppl wanting to do 386 -> amd64, and all the other permutations [08:26] davecheney: in this case, we just want to use 386 all down the line [08:27] davecheney: so i don't think the same problems arise [08:27] davecheney: although there may well be things i don't know about [08:28] rogpeppe: i think there are wrinkles [08:28] for example [08:28] _most_ ec2 instances are amd64 [08:28] except for standard (?) and micro [08:29] so i think it is more compliated than saying 'i want this environment to have amd64 quantal machines' [08:29] davecheney, rogpeppe: this question is uppermost in my mind [08:29] davecheney, rogpeppe: and I *think* it's ok [08:29] fwereade: my initial response is to say 'juju only makes amd64 machines' [08:29] and ignore the arch on the host machine [08:30] davecheney, certainly we must ignore the arch on the host [08:30] so bootstraping from anything, osx runing on arm produces amd64 machines [08:30] fwereade: then that might solve rogpeppe 's question [08:30] davecheney, I think that the tighest constraint we have is what arches we have the tools built for [08:30] fwereade: i'd be relieved to say, for the future that we can predict, amd64 is all we do [08:30] davecheney: what about --upload-tools ? [08:31] rogpeppe: fuk [08:31] double shit [08:31] davecheney: that's my motivating case at the moment [08:31] fwereade: mornin' BTW. hope you're feeling a little better. [08:31] davecheney, I don't think there's a compelling reason to drop i386 support on ec2 [08:32] rogpeppe, heyhey, pretty crappy tbh, I will be off and on [08:32] fwereade: apart from it being a wrinkle we don't need [08:32] davecheney: i'm not sure i see the problem. [08:32] davecheney: currently the logic chooses the image based on the arch and series of the tools found. [08:32] rogpeppe: it's just more perumutations that we don't need righ tnow [08:33] davecheney: i *think* all we need to do is provide a mongo db binary and it'll just work [08:33] rogpeppe: i don't think we need to do that [08:33] we should always request an amd64 image [08:33] if it happens that the development tools are 386 [08:34] davecheney: not if the caller wants to upload tools [08:34] that isn't a problem on linux [08:34] davecheney: and have them run on the bootstrap machine [08:34] yeah, that works fine [08:34] just clamp the AMI request to amd64, job done [08:35] davecheney: ah, because you can always run 386 binaries ok [08:35] * dimitern rebooting [08:35] yeah [08:35] going the other way, amd64 tools on a t1.micro will be sadface [08:35] but that is a bridge to cross at another time [08:35] davecheney, is that any worse than *anything* on a t1.micro? ;p [08:36] exactly [08:36] it's not the way the market is heading [08:36] 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] rogpeppe: --upload-tools is the hack [08:36] i'm fine with hackig it some more [08:36] rogpeppe: is there an issue for this work ? [08:37] davecheney: yeah, wait a mo [08:37] davecheney: https://bugs.launchpad.net/juju-core/+bug/1130209 [08:37] <_mup_> Bug #1130209: we need a mongodb compiled for i386 < https://launchpad.net/bugs/1130209 > [08:38] davecheney: but that's fairly specific - i assumed it was the right solution [08:38] rogpeppe: mind if I rename that issue to what I see is the problem ? [08:39] davecheney: ok [08:39] davecheney: which is? [08:41] "juju should always request an amd64 AMI image" [08:41] actually, that will probably mean we can't boot t1.micros [08:41] but whatever [08:42] davecheney, I *think* that we can start any ec2 instance with AMD64 [08:42] davecheney, but I'm not quite convinced this is an ok shortcut [08:43] fwereade: why not ? [08:43] davecheney, haven't people already used juju on ARM machines? even if they don't run the state server? [08:43] fwereade: i've made some replies to https://codereview.appspot.com/7326052/ [08:44] fwereade: only as a client [08:44] my view of the crystal ball shows only environments containing amd64 machines [08:44] when highbank or calexeda come asking, we fix the problem then [08:44] davecheney, what about the arm stuff that had to be added to python juju for maas? [08:45] 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] fwereade: in the spirit of delivering 13.04, we have to leave the goal posts in the ground [08:45] davecheney, I do not believe I am the person trying to move them... [08:45] davecheney: what *is* the problem with just uploading 386 mongo binaries, BTW? [08:46] rogpeppe: well, 1, we don't have those binaries, so that is work [08:46] davecheney: i pointed you at some [08:46] 2, getting the deps in the archive is going to be an amazing ballache, so i'd rather no x2 it [08:46] davecheney: i don't understand - which deps in which archive? [08:47] rogpeppe: with respect, I have proposed a solution that has a low cost now, and a known cost in the future [08:47] we don't have to gold plate this [08:47] we just have to make it work on jam's dev machine, right ? [08:47] davecheney: if i'd had the credentials, i could have fixed this with a single s3 upload last night [08:48] rogpeppe: how can I send them too you ? [08:48] i'm more than happy for someone else to fix this [08:48] davecheney: i have the ones you gave me before, but they don't seem to work any more [08:49] yeah, niemeyer reset them a few times trying to get the permission junk working [08:49] davecheney: ah [08:49] rogpeppe: email me your public key [08:49] i'll bootstrap a machine [08:49] and leave them there for you [08:51] 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] fwereade: well, surely the *place* is right - it's just the code is wrong? [08:52] rogpeppe, the code cannot be correct while it's in that place, though, can it? [08:52] fwereade: the code cannot be correct in any place! [08:53] fwereade: 'cos it's buggy [08:53] rogpeppe, yeah, but the bugs are intractable while it's outside the state package [08:53] fwereade: do you think we should just delete set and get support because they're currently wrong? [08:54] rogpeppe, I think that picking something that's wrong as a demonstration of how to do something right is problematic [08:54] 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] 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] 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] fwereade: the benefit of the extra layer is at least we're not duplicating the buggy code [09:00] fwereade: because it needs to be called from two places now [09:01] rogpeppe, er, what? why would we be duplicating it if we put it in one place, ie in state? [09:01] rogpeppe, both those places use the state API though, right? [09:03] 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] fwereade: i thought about putting them in state too [09:04] fwereade: it depends whether we feel that almost all the code in juju/cmd should actually be done in state. [09:04] cmd/juju [09:06] rogpeppe, well that case obviously has to be; and offhand I can't think of one that shouldn't, can you? [09:07] fwereade: so what should i say to the GUI folks? hold off while we refactor cmd/juju ? [09:08] rogpeppe, I would say "put it in state, like this"... maybe I'm not understanding some of the constraints? [09:09] rogpeppe, blast, sorry, need a quick break, can we pick this up a bit later? [09:09] fwereade: ok [09:40] 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] fwereade: shall we G+? [09:40] rogpeppe, sgtm, will you start? [09:40] fwereade: ok [09:43] fwereade: ha, i've made a hangout, but the window has disappeared [09:43] * fwereade tries [11:00] jam, ping: https://codereview.appspot.com/7307128/patch/1/6 [11:01] jam, for your first comment, I thought it was covered by "// Environ constraints are not available before initialization." just above? [11:01] jam, and for the second: yes to the first bit, clarify please to the seconds [11:02] 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 === teknico_ is now known as teknico === edamato is now known as edamato-afk [11:32] jam, mgz: standp === mohits1 is now known as mohits [12:14] 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/ === edamato-afk is now known as edamato [12:29] rogpeppe, am I right in thinking there's no api.IsNotFound? [12:32] rogpeppe, also, who should I be talking to to figure out the deal with yaml-in-the-gui? [12:33] rogpeppe, because I'm still pretty strongly -1 on the two-parallel-paths-not-checked-on-the server bit [12:49] 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] fwereade: hmm, sorry missed your remarks above [12:50] fwereade: from the top: [12:51] fwereade: yes, there's no api.IsNotFound - it's spelt api.ErrCode() == api.CodeNotFound instead [12:52] rogpeppe, how about something more like api.ErrHasCode(err, api.CodeNotFound), perhaps? [12:53] rogpeppe, just bikeshedding really, but I thought that IsNotFound was pretty handy [12:53] fwereade: in #juju-gui, bac or gary_poster are two possibilities [12:53] rogpeppe, cool, ty [12:53] 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] rogpeppe, ok, fair enough on that, not sure I really like my suggestion [12:54] fwereade: for one thing, in tests, c.Assert(api.ErrCode(err), Equals, api.CodeNotFound) gives a nicer error message on failure [12:54] fwereade: also, you can do switch code := api.ErrCode(err); code { [12:55] rogpeppe, ok, sgtm, thanks :) [12:55] fwereade: np [12:55] fwereade: FWIW i don't think the two parallel paths thing is really a problem - i just documented that Config overrides Options. [12:56] fwereade: but there's an issue around that that i was discussing with niemeyer yesterday (in your absence) [12:56] 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] 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] rogpeppe, isn't an empty string "fall back to the default" in this context? [12:57] fwereade: if only js had decent yaml support... (if only yaml wasn't insane...) [12:57] rogpeppe, ha, yeah [12:58] fwereade: yes, but it could also be "set this to the empty string" [12:58] fwereade: and in yaml you can express that difference [12:58] fwereade: my inclination is to always make an empty string equivalent to null [12:58] fwereade: then the two paths are equivalent [12:59] rogpeppe, yeah, I think we kinda decided that a while ago, but that might just be in informal inference I made [12:59] fwereade: FWIW niemeyer thought that it was reasonable to have yaml be more expressive. [13:00] fwereade: You've got a +1. [13:00] fwereade: BTW if it wasn't for the js stuff, i would have no hesitation at all about dropping the yaml path [13:00] fwereade: And thx for your review, the change is proposed again. [13:00] 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] TheMue, tyvm, I'll take a look in a bit [13:05] fwereade: i just had a little chat in #juju-gui - here's the result: http://paste.ubuntu.com/1690301/ [13:07] 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] fwereade: alternatively, we could have two entry points: ServiceSet and ServiceSetYAML [13:08] fwereade: under the surface they'd invoke the same thing [13:09] rogpeppe, I think I'd rather separate them like that if nothing else === gary_poster|away is now known as gary_poster [13:09] fwereade: ok, although i still don't really understand the problem [13:10] rogpeppe, or... they could just JSON it in the GUI, and that's still valid YAML, right? [13:10] rogpeppe, so long as we handle ""/null consistently [13:10] fwereade: true. i suppose my ickiness head goes "json-in-json-string, yeuch" [13:11] rogpeppe, I think that with the YAML requirement we're already way over that line [13:12] 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] rogpeppe, can you live with two separate calls? [13:16] rogpeppe, that seems to me like the least polluting approach possible ;) [13:17] fwereade: yeah, i'll do two separate calls. [13:25] rogpeppe, cool, tyvm [13:26] TheMue, I'm wondering about the 301s on paths with .. [13:26] TheMue, surely if anything's a 403, it's attempts to read/write outside the path? [13:27] TheMue, and a prefix of "../" acting the same as no prefix is a bit weird as well I think [13:27] TheMue, when listing I mean [13:29] fwereade: I've wondered too and it took me some time to discover it. [13:29] fwereade: It's a feature of the http daemon of Go. [13:29] fwereade: It resolves relative paths and sends a redirect to the client. [13:30] fwereade: And I also wondered about why using it with a pattern doesn't lead to a 301. Strange. [13:32] TheMue, hmm, where does it redirect to? [13:33] fwereade: I have to check now that the URL doesn't contain the environ name anymore. [13:34] fwereade: Before it has been from /environname/../foo to /foo [13:34] fwereade: But only a redirect, I client has to interpret it itself. [13:35] 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] 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] fwereade: Right now we don't do any authentication on the storage. [13:36] TheMue, so where does localhost:60006/../foo redirect to? [13:38] 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] fwereade: If that does then react on it and does a request and the file doesn't exists it gets a 404. [13:39] 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] fwereade: But give me some minutes, now I'm interested in that redirect too. [13:39] TheMue, but nonetheless I would rather have a PUT to ../wherever/blah *fail* rather than just put *somewhere* ;) [13:39] TheMue, thanks [13:40] 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] fwereade: And security has nothing to do with the generated redirect, here we should introduce a real authentication feature. [13:41] fwereade: At least basic auth. [13:43] TheMue, what scenarios are we protecting against here? [13:44] fwereade: One moment, testing. [13:45] 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] 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]