[00:05] bugs, so many bugs [00:40] hi bigjools [00:40] o/ [00:40] OTP [00:40] np, just saying hello [00:40] hello bac :) [01:05] I need some guidance here: http://bazaar.launchpad.net/~binary132/juju-core/charm-actions/view/head:/charm/actions.go [01:05] this bit in particular (line 58): actionsSpec.ActionSpecs[name].Params = map[string]interface{}{} [01:06] I'm trying to initialize the Params member with an empty interface (I've also tried make( map[string]interface{} ) with the same results but it's giving me the following error: [01:07] looking for a review on this [01:07] https://github.com/juju/errgo/pull/6 [01:07] wallyworld_: nate, jimmiebtlr_ [01:07] cannot assign to actionsSpec.ActionSpecs[name].Params [01:07] no, jam [01:07] presumably because Params is an uninitialized map [01:08] bodie_: i don't think reflect is necessary [01:08] you can do equality on a potentially nested map without reflect? [01:08] if actionSpec.ActionSpec[name].Params == nil { actionSpec.ActionSpec[name].Params = make(map[string]interface{}) [01:08] davecheney: lgtm, i like it [01:08] } [01:09] I tried that, but it gives me the same error [01:09] it doesn't want me to assign values to that member... I'm not seeing why [01:09] because maps are not addressible [01:10] bodie_: why do you need to initalise that map [01:10] line 60 will work fine with the zero value [01:10] well, I don't necessarily need to. what's happening is that I'm expecting an empty map in my test but getting nil [01:10] which got me thinking [01:11] var actionsSpec *Actions [01:11] if err := goyaml.Unmarshal(data, &actionsSpec); err != nil { [01:11] perhaps if the "params" key has no values, it should be defined as an empty value, rather than nil, that way it can be handled without nil checks [01:11] ^ ??? [01:11] what's the question [01:11] your passing **Action to Unmarshal [01:11] shouldn't this be [01:11] one moment sorry [01:11] var actionSpec Actions [01:11] i'll be back [01:12] goyaml.Unmarshal(date &actionSpec) [01:15] okay, looking [01:15] child just awoke suddenly... [01:16] mh [01:16] hm* [01:18] wallyworld_: hey. are you on mgz's box? [01:18] no [01:18] k [01:19] wallyworld_: follow up for ya https://code.launchpad.net/~dave-cheney/juju-core/178-update-errgo-and-loggo-dependencies/+merge/220340 [01:19] rightio [01:21] land that sucker [01:21] wallyworld_: are you looking at the bzr error, or should I? [01:22] axw: i haven't looked yet [01:22] it did happen after my branch landed [01:23] hrm ok [01:23] but off hand i can't see the causal link [01:23] I wonder if it's due to the bzr config not being found in $HOME [01:23] leave it and doing another one if you want, likr the new mongo failure [01:23] ok [01:23] maybe [01:23] axw: but it does pass here locally [01:23] I see [01:23] so could be pythin version related according to curtis [01:25] davecheney, seems weird that it would work even though I had the error you pointed out [01:25] anyway, corrected that but it's still giving me the same thing [01:26] with make as well as the way I had it [01:27] axw: it does seem that in CI, that ensure admin user stuff is happy now [01:27] hooray [01:27] but of course we have the new failures :-( [01:27] these last couple have been a massive PITA [01:27] yeah [01:27] faaaaaaaaaaaaaaark [01:27] https://code.launchpad.net/~dave-cheney/juju-core/178-update-errgo-and-loggo-dependencies/+merge/220340 [01:27] this is really gettig me down [01:27] never ending [01:27] yep :-( [01:27] bodie_: can you paste the error [01:27] please [01:28] sure thing [01:28] cannot assign to actionsSpec.ActionSpecs[name].Params [01:28] bodie_: i need the code, the line number and the error [01:28] davecheney: I think that means the bot needs manually updating [01:28] davecheney: i'm not a git expert so have no idea wtf that error means [01:29] wallyworld_: what axw said [01:29] wallyworld_: godeps -u doesn't actually pull new revs [01:29] axw: nope, because godeps creates disconneted heads [01:29] so godeps needs fixing? [01:29] which is a git word for, i dunno, branches that have no upstream [01:29] wallyworld_: yup [01:29] i can manually update the bot [01:29] wallyworld_: thanks [01:30] davecheney: can you file a bug for the godeps thing? [01:30] davecheney, charm/actions.go:58: cannot assign to actionsSpec.ActionSpecs[name].Params [01:30] http://bazaar.launchpad.net/~binary132/juju-core/charm-actions/view/head:/charm/actions.go [01:32] same error if I use a composite literal of map[string]interface{} [01:33] er... append {} [01:34] bodie_: ok [01:34] sorry [01:34] there are two things going on [01:34] 1. map values may not be assignable [01:34] but [01:34] 2. you probaly don't need to do all that defensive coding [01:34] * davecheney reads the code [01:34] bot fixed, tests running [01:35] wallyworld_: got another one for you https://code.launchpad.net/~dave-cheney/juju-core/179-remove-thirdparty-pbkdf2-dependency/+merge/220341 [01:35] ok [01:35] I don't necessarily want to assign anything except an empty but non-nil value [01:36] just so I don't have people stumbling on nil param sets if the action doesn't take any parameters [01:36] I had a devil of a time trying to figure this out earlier [01:36] but, I guess I can leave it nil -- I just don't want inability to be what prevents me from doing it right [01:36] frustrating [01:37] bodie_: leave it as nil [01:38] bodie_: i'm really not sure about all the use of reflect in that file [01:38] it shouldn't be necessary [01:39] in fact, why won't that struct just unmarshall directly ? [01:39] bodie_: dont you just need to add some `yaml:...` tags to the ActionSpec struct [01:39] and it should unmarshal directly [01:39] without having to do it by hand [01:40] i'm clearly missing something [01:42] I'm probably not fully understanding the difference between what you're suggesting and what I have here -- I'm not certain I can do it justice, but I've been over it with William several times and he thought it important to leave room for metadata that we specify, in addition to a more flexible Params map per action which only need conform to JSON-Schema [01:43] bodie_: can you show me a sample actions.yaml file [01:43] the deep equality was because the compiler complained about the use of normal equality, and it seemed sensible since there could be nested maps [01:43] yeah, there's an example in my mr... let's see [01:44] https://codereview.appspot.com/94540044 [01:44] bodie_: it feels like you are fighting the language [01:44] perhaps a bit :) [01:44] and you'll loose, 'cos its really stubourn [01:44] so i'd like to help find a better way to do what you want to do, rather than help you win a small battle with the compiler [01:44] that usually means I'm not properly understanding the way it wants me to do things :) [01:45] so looking at that definition [01:45] you wnt to do somethign like [01:46] type ActionSpec struct { Description string ; Params map[string]interface{} } [01:47] actionSpecs := make(map[string]ActionSpec) [01:47] goyaml.Unmarshal(r, &actionSpecs) [01:47] that should be pretty close [01:48] then you'll need to go through the params of each ActionSpec to do the json validation [01:48] but that should be a separate function [01:48] * davecheney doesn't even know what json validatoin means [01:48] it sounds like an oxymoron [01:48] json-schema is a subset of json which has to conform to a spec [01:48] it's to define the format of json objects which will be passed as the arguments to the action [01:49] invalid params will be rejected [01:49] and you can just check it against the JsonSchemaDoc to validate the parameters [01:49] this is the argument in favor of this [01:49] sure [01:50] but that doens't need to be wedded to the yaml decoding process [01:50] so, we're ensuring that this yaml matches the format we need for json-schema, since charm coders like yaml, or something [01:50] it should just be a function that takes a map[string]interface{} and returns an error [01:50] arosales: did you end up meeting with the joyent folks? or is that yet to happen? i don't think i need to be there - hopefully the pull requests are self explanatory [01:52] I think the goyaml.Unmarshal bit is making the map, but I could be wrong [01:59] davecheney, we're just making sure at the time of unmarshaling that it's valid [01:59] the jsonschema bit just checks that, does nothing with the result [02:07] then, I'm going through each action in the list, making sure it has an OK name, and then checking to make sure its params do [02:07] that's really all [02:07] if the params map is nil, I'm trying to make it empty instead [02:21] * davecheney twiddles thumbs waiting for the bot [02:40] jam: you put up a branch fixing this issue before, right? but it didn't get proposed for trunk? #1320794 [02:40] <_mup_> Bug #1320794: After bootstrapping an OpenStack environment, juju tries to connect to the internal IP

[04:46] wallyworld: I haven't meet with them yet, but I think they are working on the pull requests [04:47] arosales: great, thanks, let me know if i can do anything [04:47] wallyworld: np will do [04:54] axw: IIRC we put one together that made it so we didn't use networks marked MachineLocal, however, it is possible the auto-detection in Openstack is wrong. [04:54] I don't think I did the fix [04:58] jam: ok. thought you did something for ODS [04:58] maybe that was it [04:58] axw: ah the ODS thing was attached to a bug but never proposed because it didn't have any tests. [04:59] axw: that was https://bugs.launchpad.net/juju-core/+bug/1308767 [04:59] <_mup_> Bug #1308767: juju client is not using the floating ip to connect to the state server

[05:00] it does have a branch associated, but without tests or someone at least trying it in the wild, I wasn't confident about it. [05:00] you're welcome to pick it up and finish it off [05:00] it *seems* correct to me [05:00] jam: thanks, that's what I was thinking about. I will take a look after my current work [05:05] jam: do you have any idea what might cause "bzr add" to crash hard and print out "(PC=0x4143E6)" [05:06] wallyworld: that looks like a stack trace, which would sound like a C sort of extension, which *sounds* like a plugin. [05:06] try "bzr add --no-plugins" ? [05:06] well s/stack trace/core dump/ [05:06] jam: this is on jenkins, in some of the tests [05:07] bug 1321282 [05:07] <_mup_> Bug #1321282: PANIC: branch_test in trusty

[05:07] i guess we could modify the tests [05:07] to add the --no-plugins arg [05:07] wallyworld: so the PC=... stuff is Go [05:07] not bzr [05:08] i could have msiread the source code. it looked like when the cmd.exec failed it printed the failing cmd line and then the stdout [05:08] i'll look closer [05:09] wallyworld: output is a []byte, so maybe it is giving the array rather than the string content? [05:09] seems strange that it's just the bzr commands, hence my suspicion based on no facts that bzr might be involved [05:09] could be yeah [05:09] wallyworld: my guess is that the failure is actually that bzr isn't present [05:09] note that we aren't checking "err" [05:09] which could be saying "ENOENT" [05:10] yeah, true [05:10] i'll modify the test to give better output [05:10] i haven't really looked that closely [05:42] does anyone know anything about a Tagger interface ? [05:42] for things that have, uh, a tag ? [05:52] davecheney, I think all state entities have one [05:53] davecheney, the tag is the definitely-unambiguous identifier we use over the API [05:53] davecheney, ie unit-mysql-3, machine-7-lxc-1, service-blah, relation-some-horrible-mess [05:54] davecheney, except we don't always use it over the API, and sometimes for no explicable reason someone allows them to leak into the UI [05:54] fwereade: there is no state.Tagger interface definition [05:54] nm, i just made one up [05:56] fwereade: i can easily move that definition into the state package for better feng shui [06:37] davecheney, heh, it's state.Entity [06:46] fwereade: right [06:46] will fix [06:46] or I can fix in a followup [07:31] morning [07:51] hey TheMue [07:53] wallyworld: FYI, bzr tests are barfing on my laptop [07:53] oh great :-( [07:53] wallyworld: I can take that next if you're not on top of it [07:54] axw: i've been caught up with other stuff o haven't yet got root cause sussed out. i'll take another look and we can discuss at standup [07:55] bug 1321009 is pretty important [07:55] wallyworld: sure [07:55] wallyworld: is that what you're looking at now? [07:55] wallyworld: or I should take a look? [07:55] nah, gccgo stuff [07:55] ok [07:55] feel free [08:00] morning all [08:03] morning voidspace [08:04] (just how it sounds :) [08:06] axw: i just got the failing bzr tests as well although i could have sworn they passed before as i did a complete test run. go figure [08:06] hey all [08:07] I'd love a review of https://codereview.appspot.com/94540043/ [08:07] wallyworld: me too, though I did a pull earlier... I'm pretty sure I had all of your changes already though [08:11] fwereade, jam, vladk|offline, I'd really appreciate a second look on this goamz CL https://codereview.appspot.com/98430044/ [08:11] * dimitern is away for a while [08:29] fwereade, hi [08:30] tasdomas, heyhey [08:30] dimitern, oops, looking [08:32] tasdomas, (I can talk and review at the same time, at least to some degree) [08:32] fwereade, was thinking about the ensure-availability MP - should we display the Promote/Demote/Remove information always or just in verbose mode? [08:33] wallyworld: I think it will be very difficult to backport that fix for 1.18 [08:33] it relies on a bunch of existing changes in 1.19 that allow dialling multiple addresses [08:33] axw: ok, it seemed like something we would have wanted [08:33] tasdomas, I think we should generally display feedback, and have a --quiet option; --verbose remains meaningful, it just means *loads* of output [08:34] axw: i think a 2nd opinion to mine as to how important it is would be desireable [08:34] wallyworld: ideally yes, but I think it should probably just wait for 1.20 unless it's critical (which I assume it's not, since this is nothing new) [08:34] ok [08:43] fwereade, so would this sort of output look sane to you: http://paste.ubuntu.com/7496511/ ? [08:46] axw: FFS :-( https://codereview.appspot.com/100590048 [08:56] wallyworld: aha [08:57] wallyworld: what's the reason? [08:57] mgz: i was dumb [08:57] mgz: bzr wants a home [08:57] it tries to read /.bazaar if $HOME=="" [08:58] oh yeah, \bsse just removes the envvar [08:58] wallyworld: mgz sorry to be a pain, going to be a little late again - gotta get my daughter to have a shower... back real soon [08:58] yep [08:58] ok, np [09:09] tasdomas, sorry: I'd say --quiet can suppress all output, and it would be perfect if normal mode output the final list of expected sate servers [09:10] fwereade, ok - what do you mean by "final list of expected state servers" ? [09:11] tasdomas, if we add a new one it isn't a state server *yet*, but we expect that in a couple of minutes it will be [09:12] wallyworld mgz: back [09:15] fwereade, so instead of printing the promotion/demotion lists, I should just print a list of what state servers are expected to be operating once the ensure-availability process completes (i.e. collapse those lists)? [09:16] tasdomas, yeah, I reckon the lists are maybe best for --verbose [09:17] tasdomas, the thing most people care about is "which ones are now my state servers" [09:17] tasdomas, with an option on "which ones used to be but aren't any more" [09:18] fwereade, ok - I will update the branch [09:42] fwereade, how's the looking going? :) [09:43] * fwereade glances guiltily at dimitern, changes tabs [09:45] :) [09:45] np [09:45] wallyworld: I wonder if we should switch over goose or something first, less active than core and may avoid some early pain [09:47] mgz: not a bad idea. but i guess we already have other things in github, but not the landing bot support. let's see how we go over the next day or so. at this stage i think we should just rip off the band aid [09:47] also, the guys getting store out of core are waiting for us [09:48] gotta run though, back later maybe [09:48] wallyworld: goose is on the landing bot atm I mean, so we need both and it's the same steps [09:48] just doing the smaller one first will give a day or whatever to find glaring issues [09:50] router firmware update [09:50] going offline briefly [09:53] dimitern, so, it is looking pretty sane to me, but I'm worried I'm not the best person to review it -- jam, do you feel ~at home with the ec2 testing double? I've never really worked with it at all [09:56] dimitern, all I really have is a whine that one of the methods is awfully big [09:57] fwereade, it is indeed, and that's after splitting it up a bit [09:57] fwereade, I welcome suggestions how to simplify it further though [09:58] dimitern, I was thinking that at least the if len(ifaces) == 0 block could move out [09:58] dimitern, it was less clear whether it'd be a win to extract that massive switch [09:58] fwereade, ah, good point [09:59] fwereade, it's really not that easy to parse that huge complicated form [09:59] dimitern, I suspect that if you were to do that it'd be a matter of a new type that sort of accumulates state in its fields [09:59] fwereade, hence the switch [09:59] fwereade, expand a bit please? [10:02] dimitern, some of those switch branches are pretty big themselves, so it could be good to extract them; but maybe it isn't even a type, it's just a func that takes a *iface and modifies it in place [10:02] sorry forget anything I said about an extra type [10:03] dimitern, just consider breaking the longer switch branches out so it's easier to see the structure of the whole method without getting lost [10:03] fwereade, i kinda feel there is indeed a type waiting to emerge around parsing stuff in the test server, but it will require bigger refactoring [10:03] fwereade, alright then, sgtm [10:04] dimitern, I heartily support the "refactor first, so that the change you want to make becomes trivial" approach, so I'm not going to complain if you do do that, but if it's not clear what to do maybe best to skip it ;p :) [10:08] fwereade, yeah, it will blow up the CL without much immediate gain, and I just want to get it landed so I can concentrate on juju work [10:08] dimitern, +1 === vladk|offline is now known as vladk [10:13] hi, I'm having some issues trying to deploy charms on canonistack; I've filed 2 bugs in case someone can help me figure out what's wrong with this [10:13] https://bugs.launchpad.net/juju-core/+bug/1321686 [10:13] <_mup_> Bug #1321686: initial deploy using local provider fails because template creation takes too long [10:13] https://bugs.launchpad.net/juju-core/+bug/1321683 [10:13] <_mup_> Bug #1321683: juju takes a long time to deploy using the local provider [10:25] is `juju ensure-availability` supposed to work with the local provider? [10:27] tasdomas: it is currently blocked from working there [10:27] well, it doesn't work there [10:27] enabling it properly is a big pile of work [10:27] natefinch: https://codereview.appspot.com/94510043/ [10:27] doing an approximation of it is just a matter of disabling some checks [10:29] jam, just to check that I understand this correctly - enabling it would mean changing the location of the state server in local provider (as I understand it, localhost is the state server in local provider) [10:30] tasdomas: enabling it would mean that we would add a state server also in a container. To enable it properly that state server would then need something to talk to running on your host that could start more containers [10:30] because from within a container, you cannot spawn other containers [10:31] jam, thanks [10:48] fwereade: want to join our standup? [10:49] jam, why not :) [10:49] https://plus.google.com/hangouts/_/canonical.com/juju-sapphire [11:01] natefinch: actually there was a review from fwereade that I missed - with a few changes needed on that mp [11:01] natefinch: so switching to completing that [11:02] ok cool [11:16] fwereade: ping [11:16] voidspace, pong [11:16] fwereade: about your review comments on the rsyslog branch [11:17] fwereade: specifically, dropping the Port field on RsyslogConfig [11:17] fwereade: I did start down that road [11:17] voidspace, oh yes? [11:17] fwereade: it means having RsyslogWorker creation plus template rendering use HostPorts instead of the global Port [11:18] voidspace, yeah, that was the plan in my mind [11:18] unfortunately at bootstrap time we have a global port and not a set of HostPorts available [11:18] so it means threading this api change through several layers of architecture in two places (initial creation plus in the worker handle) [11:18] that was quite an invasive change (not *too bad*) [11:18] https://code.launchpad.net/~mfoord/juju-core/ha-rsyslog-api/+merge/220087 [11:19] I made the code changes, but that broke a metric crapload of tests *too* that would also need fixing [11:19] (and I'm a little uncertain about what I've done with worker creation - but I can talk that through with someone) [11:20] fwereade: so it seemed like quite a lot of extra work for something we *might* use in the future (switch away from a global port) [11:20] fwereade: so in discussion with natefinch it seemed like defering that work until we actually needed this capability was more sensible [11:20] fwereade: would you rather I spent the time on it now? [11:20] voidspace, heh, I see -- tbh those changes do look like a win to me (although there's still the odd Port sitting around?), but let me think a sec [11:21] voidspace, the main thing is that if we have HostPorts in the API but don't use it, there doesn't seem much point to that [11:21] it's an initial step... [11:22] it means when we do switch the GetRsyslogConfig already supports it [11:22] the *API* already supports it [11:22] voidspace, right, but unless that step is imminently followed by further steps in the same direction its value is somewhat limited [11:22] well, it means when we do make the switch it doesn't require a backwards incompatible API change [11:22] voidspace, versioning is coming along apace, thanks be to jam [11:23] switching template rendering etc to use it - but still *actually* only storing and using a global port would be just as pointless [11:23] but more work [11:23] even with these changes (plus maybe pulling the Port out of syslogConfig altogether) wouldn't change the fact that we're only *really* using a global Port [11:24] it would just mask it further [11:24] voidspace, agree -- but each, in the absence of the other, is just a speculative change, and they rarely work out well [11:24] so yes, fewer changes to make in the future - but no actual change in behaviour [11:24] voidspace, getting the mask in place between the api server and the rest of the world is imo a good sort of masking [11:24] fwereade: but even with this extra work, it's *still* a speculative change [11:25] fwereade: right, yep [11:25] that is true, it leaves the only place that thinks in terms of a global port being the internal database [11:25] voidspace, it eliminates the assumption of a single port among the api clients [11:25] fwereade: it sounds like you'd rather I did it [11:25] voidspace, what happens under the covers is exposed to far fewer people ;) [11:26] voidspace, I would, yeah [11:26] ok [11:26] np [11:26] voidspace, tyvm [11:27] sorry to throw a wrench in things, guys. It just sounded like more work was being added to support a feature that may or may not ever be implemented, and I wanted Michael to be able to move on to other projects. [11:35] fwereade: natefinch: shall I do this as a separate CL - it's going to be quite a big diff against the existing branch [11:35] so I can address the *other* comments from fwereade in the existing CL and then work on the new one [11:36] voidspace: that sounds like a good idea [11:39] fwereade: ah, there's a difficulty with eliminating use of Port altogether [11:39] fwereade: when we start a new state server we need to pick a port to use [11:39] fwereade: and the "State" needs to *know* which port we picked, because it needs to associate the state server with that port to return in RsylogConfig.HostPorts [11:40] voidspace, does that happen at the same time as we generate the cert? [11:40] fwereade: to be honest I'm not sure how we pick the initial port - let me check [11:43] fwereade: hmmm... jujud/agent doesn't pass a port and the worker is started with a port of 0 [11:43] voidspace, so, wait, we have an env config setting that's just ignored? [11:44] ensureCertificates doesn't set a port [11:44] * fwereade wonders why it's env config in the first place then :/ [11:45] fwereade: we must be setting it somewhere - because HA does set the correct port [11:45] I just haven't found it yet [11:46] fwereade: hah [11:46] fwereade: we have DefaultSyslogPort in environs/config/config.go [11:46] voidspace, that syllable rarely preceds good news [11:47] fwereade: what happens is that we use port 0 [11:47] fwereade: when we set the cert it causes a rewrite of the config which requests the port from EnvironConfig [11:47] fwereade: which returns the DefaultPort, which is what we use [11:47] and port 6541 is what I always see used (the default) [11:47] we have an api to change the port though [11:48] voidspace, ah, ok, but someone might have set it to something different, and in that case it would be used? [11:48] let me see where that's called from [11:48] fwereade: yes, if it's changed we would (previously anyway!) see the change and rewrite [11:48] fwereade: axw is certain that the port is in the config when the worker is created - so we should be safe (and indeed we will make the port immutable once set) [11:49] * fwereade is somewhat reassured [11:49] heh [11:50] hmmm... no, we only have an api to set the cert [11:50] setting the port has to be done through environconfig I believe [11:52] fwereade: sigh, so I don't think it's set deliberately in our code - we always use the default [11:52] fwereade: so the only way it would be set would be by users doing it - and we wouldn't pick up that change with our new code [11:52] fwereade: so I need to talk to axw again about the motivation for allowing it to be changed [11:52] fwereade: if there's a real use case then we need to still support it [11:53] he said it was because previously it could pick a privileged port and rsyslog refuse to start [11:53] but with our default of 6541 I can't see that being the case now [11:54] it maybe that the initial starting of the rsyslog worker was picking a privileged port when we give it port 0 [11:55] but as far as I can see we *always* overwrite that immediately - we don't set back the port the system gives us [11:56] hello [11:58] wwitzel3: morning [11:59] fwereade: what that means though is that we *don't* have a way to pick a port for rsyslog - we use the default or one the user has set [11:59] fwereade: so a new state server coming online either has to have a new way of picking a port, and that needs to be stored associated with the server [11:59]