[02:45] <davecheney> thumper: http://paste.ubuntu.com/12069959/
[02:45] <davecheney> this is from this morning's run
[02:45] <davecheney> summary: juju is now as stable on ppc64 as any other platform
[02:50] <thumper> hazaah
[03:35] <menn0> thumper: here's the final allenvwatcher pr: http://reviews.vapour.ws/r/2374/
[04:51] <menn0> thumper: any chance you can look at the above? ^^
[04:51]  * menn0 is keen to be done with this 
[04:53]  * thumper is looking now
[04:53] <menn0> thumper: thanks
[05:25] <thumper> menn0: done
[05:25] <thumper> one (strong) suggestion
[05:25] <menn0> thumper: ok...
[05:27] <menn0> thumper: no problems. I had something like that at one point.
[05:29] <menn0> thumper: to be fair those funcs aren't really for truly public use (hence the comment)
[05:40] <thumper> well... if they have capital letters, they're public
[06:55] <dimitern> TheMue, morning
[06:56] <dimitern> TheMue, I see you've assigned the constraints card to yourself, but you're yet to land your other branch, so I've reassigned it to myself
[07:10] <TheMue> dimitern: morning, the landing card is just waiting, that CI is unblocked
[07:10] <dimitern> TheMue, cool
[07:11] <TheMue> dimitern: so which card can I start with now?
[07:12] <dimitern> TheMue, hmm.. let me have a look
[07:13] <dimitern> TheMue, you have a follow-up to do, don't you? :)
[07:13] <dimitern> also it seems I've assigned the wrong card to myself - I'm actually working on the other constraints card
[07:13] <TheMue> dimitern: hehe
[07:15] <TheMue> dimitern: you mean "Test isNetworkingEnvironment() in ..."? we haven't moved it from backlog to iteration backlog, so I thought it's not as important as the spaces cards
[07:16] <dimitern> TheMue, yeah, we forgot about this one
[07:16] <dimitern> TheMue, the net-cli stuff is the most important once the previous tasks are done :)
[07:16] <TheMue> dimitern: ok, so I'll move it
[07:16] <dimitern> TheMue, cheers, and feel free to estimate it as well
[07:17] <TheMue> let's move it, move it, let's move it, move it *sing*
[07:17] <TheMue> dimitern: ok
[07:22] <voidspace> dimitern: ping
[07:24] <dimitern> voidspace, pong
[07:24] <voidspace> dimitern: did you write ListSubnets?
[07:25] <voidspace> dimitern: unlike all the other API methods it tages a *names.SpaceTag rather than a names.SpaceTag and I'm wondering why
[07:25] <voidspace> I changed it, as it was marginally inconvenient in one place, and a few things broke
[07:25] <voidspace> so I probably have to put it back
[07:26] <dimitern> voidspace, I did - it's because the space is optional for filtering
[07:26] <dimitern> voidspace, it was supposed to make calling the client-side method more convenient :)
[07:26] <voidspace> dimitern: and you need to be able to distinguish between not used (nil) and an explicit empty space ("")
[07:28] <voidspace> dimitern: so, changing API methods to take tags instead of strings means changing a bunch of CLI calls too
[07:28] <voidspace> dimitern: they need to use the tags and do the validation
[07:29] <voidspace> so I'm now down to only 23 test failures...
[07:29] <voidspace> (the CLI methods were already validating of course - but not always storing a tag. And it's *mostly* tests that need to change.)
[07:32] <dimitern> voidspace, :)
[07:33] <dimitern> voidspace, sounds good
[07:44] <dimitern> TheMue, voidspace, dooferlad, guys, I'm suggesting to cancel the 1:1 meetings today - happy to chat about anything, if you want of course
[07:44] <dimitern> but in the face of the feature freeze I'd rather concentrate on that with priority
[07:45] <TheMue> dimitern: frome my side nothing special, so sounds good
[07:47] <dimitern> TheMue, cheers
[07:47] <voidspace> dimitern: sure
[07:48] <voidspace> dimitern: thanks
[07:55] <dooferlad> dimitern: +1
[07:55] <dimitern> thanks guys!
[09:01] <voidspace> dimitern: TheMue: dooferlad: ready in a minute or two - coffee!!!
[09:32] <voidspace> dimitern: TheMue: dooferlad: not sure if my last message got through, my internet went down
[09:32] <voidspace> sorry
[09:32] <voidspace> I'm back now but the hangout was empty
[09:33] <TheMue> voidspace: you've been frozen and then the connection dropped
[09:33] <TheMue> voidspace: so the question is, what has been the last message? ;)
[09:33] <voidspace> that was my last message...
[09:33] <voidspace> "my internet went down"
[09:33] <voidspace> :-)
[09:34] <dimitern> voidspace, oh - I think I heard only "own" :)
[09:36] <TheMue> hehe
[09:36] <voidspace> dimitern: own?
[09:40] <voidspace> dimitern: out of "down"?
[09:42] <dimitern> voidspace, I think so yeah
[09:42] <dimitern> (well it sounded like "on")
[09:42] <voidspace> heh
[09:42] <voidspace> sorry
[09:42] <voidspace> :w
[09:42] <voidspace> wrong window...
[09:43] <dimitern> voidspace, so it seemed we should chat about subnet add/create vs tagging?
[09:43] <voidspace> dimitern: well, I thought we had talked about it a few times already!
[09:43] <voidspace> dimitern: that's why I was so surprised
[09:44] <voidspace> dimitern: previously you seemed to agree that Create was easier and we didn't need to touch tagging for the MVP
[09:44] <dimitern> voidspace, yeah, I might have misinformed you unintentionally
[09:44] <voidspace> dimitern: if we *have* to do Add, then so-be-it, but I think it's *more work*
[09:44] <dimitern> voidspace, we'll implement create, but not include in the MVP
[09:45] <voidspace> sure, I'm talking about what is the least work we can do which gets something useful
[09:45] <dimitern> voidspace, also add doesn't need to respect pre-existing tags on a subnet in ec2
[09:45] <dimitern> for the MVP
[09:45] <voidspace> so what does it do?
[09:45] <dimitern> voidspace, it adds it to state, validating it exists on the provider
[09:45] <voidspace> just assumes the subnet exists and adds it to state
[09:46] <voidspace> so it does go out to the provider
[09:46] <voidspace> (the current implementation doesn't...)
[09:46] <dimitern> voidspace, yes, as we have already a Subnets call we can call - it shouldn't be too bad to add this check
[09:46] <voidspace> ok
[09:46] <voidspace> so if we ignore tagging then Add is easier :-)
[09:48] <dimitern> voidspace, indeed - we'll add tagging (discovery and updates) later if we manage - it's strictly an improvement in UX and observability, not so much in functionality
[09:52] <voidspace> dimitern: fair enough I guess
[09:52] <voidspace> dimitern: like I said, based on our previous conversations I was just surprised
[10:12] <voidspace> dimitern: updated PR http://reviews.vapour.ws/r/2360/
[10:25] <dimitern> voidspace, cheers, looking
[10:31] <dimitern> dooferlad, others: http://reviews.vapour.ws/r/2376/ spaces constraints
[10:31] <dooferlad> dimitern: already reading :-)
[10:34] <dimitern> dooferlad, ta!
[10:35] <dimitern> voidspace, didn't we talk about ListSubnets returning something like []params.SubnetInfo?
[10:36] <dimitern> voidspace, I mean the client-side api method
[10:37] <voidspace> dimitern: I don't recall a conversation about that, let me look and see what it does return
[10:37] <voidspace> it's been a while since I wrote that code
[10:38] <voidspace> dimitern: hmm... it returns a list of params.Subnet
[10:39] <voidspace> dimitern: there is no params.SubnetInfo is there?
[10:39] <dimitern> voidspace, sorry, I meant params.Subnet
[10:39] <voidspace> dimitern: well, it has a result type that holds a list of params.Subnet
[10:39] <dimitern> voidspace, well, looking at your branch it returns ListSubnetsResults and an error
[10:39] <voidspace> dimitern: do you mean removing the ListSubnetsResults part from the returned value
[10:40] <dimitern> voidspace, that's pretty unusual for client-side apis
[10:40] <dimitern> voidspace, just return the Results part of it and the error
[10:40] <voidspace> dimitern: ah
[10:40] <voidspace> dimitern: ListSpaces does the same
[10:41] <voidspace> dimitern: ah, and the CLI expects just a list of subnets
[10:41] <voidspace> dimitern: so that needs fixing
[10:41] <voidspace> probably the same for ListSpaces too
[10:41] <voidspace> dimitern: thanks
[10:43] <voidspace> dimitern: just return response.Results or can we get rid of the response type completely and just return a slice of Subnet ?
[10:44] <voidspace> (I mean can the apiserver return []params.Subnet or do we need a Result type to hold it)
[10:46] <voidspace> I'm just changing the api List* methods
[10:46] <dimitern> voidspace, nope, the apiserver-side still needs to use the result type like this
[10:46] <voidspace> yeah
[10:46] <dimitern> voidspace, you have a review
[10:46] <voidspace> dimitern: thanks
[10:47] <voidspace> dimitern: why do we need to verify that the command members are set correctly on error?
[10:47] <voidspace> dimitern: given that we report the error to the user and exist
[10:48] <dimitern> voidspace, I'd appreciate a second look on my review as well - esp. if any of the TODOs reminds you of something I might've missed
[10:48] <voidspace> dimitern: ok
[10:49] <dimitern> voidspace, it's not a big deal to leave this in place
[10:49] <dimitern> voidspace, is it?
[10:51] <voidspace> dimitern: well, as we're now using SubnetTag I can't construct a SubnetTag for an invalid CIDR
[10:51] <voidspace> dimitern: so in the case of error we can't check that
[10:52] <voidspace> dimitern: and if there's no *need* for the others then it's better not to test them
[10:52] <dimitern> voidspace, ah
[10:52] <voidspace> we should only test behaviour we actually need
[10:52] <dimitern> voidspace, sure, then it's fine
[10:52] <voidspace> thanks
[10:52] <dimitern> voidspace, unlike APIs, those CLI methods are not usable from outside the package
[10:53] <voidspace> yep
[10:53] <voidspace> that was my thinking
[10:53] <voidspace> dimitern: what's the value in changing providerId to network.Id?
[10:53] <voidspace> dimitern: there's no validation, so it doesn't really offer any benefit like SubnetTag and SpaceTag do
[10:54] <voidspace> it can take arbitrary strings
[10:54] <voidspace> I believe
[10:54] <voidspace> and it probably means finding and fixing another crap-load of tests :-p
[10:54] <dimitern> voidspace, fwereade reminded me on more than one occasion that we should use typed strings for foreign ids, like the provider ones
[10:54] <voidspace> ok
[10:54] <voidspace> fair enough
[10:54] <voidspace> I'll bite the bullet
[10:55] <dimitern> voidspace, yeah, no validation for now, having a type makes adding this easier
[10:55] <dimitern> if/when needed
[10:55] <dimitern> voidspace, thanks
[10:55] <voidspace> so for our imaginery future use case then
[10:55] <voidspace> ;-)
[10:56] <dimitern> well :)
[10:56] <voidspace> dimitern: network.Id is used for too many *different* things for us to ever be able to add meaningful validation
[10:56] <voidspace> we'd have to create a *specific* type
[10:56] <dimitern> good design and architecture includes some need for precognition and such
[10:57] <dimitern> voidspace, it's used for provider-specific ids
[10:57] <voidspace> dimitern: many *different* provider-specific ids with different specifications
[10:58] <dimitern> voidspace, at least having them marked as such will simplify changing them to separate types if needed IMO
[10:59] <dimitern> voidspace, I hope you don't mind too much for a string cast here and there :)
[11:01] <voidspace> dimitern: I'm not totally convinced about the *semantic* value of using types (that offer no tangible other benefit) to represent "things" that actually *are* strings, and having to case to and from everywhere they're used
[11:01] <voidspace> dimitern: I'll do it, I'm just not convinced it adds value
[11:01] <dimitern> voidspace, I'll wait for your and mine branches to land and then create the net-cli-mvp branch
[11:02] <dimitern> voidspace, right
[11:02] <voidspace> dimitern: cool
[11:03] <voidspace> dimitern: I'll take lunch shortly
[11:03] <voidspace> I'll try and get the branch landable first
[11:04] <dimitern> voidspace, ta!
[11:25] <voidspace> dimitern: right, made those changes
[11:25] <voidspace> dimitern: switching to network.Id touched 6 files, so you might want to take another brief look
[11:25] <voidspace> :-D
[11:25] <voidspace> maybe "want" is the wrong word
[11:26] <dimitern> voidspace, sure, looking
[11:27] <dooferlad> dimitern: https://github.com/juju/juju/compare/net-cli...dooferlad:net-cli-add-deployment-support-for-spaces?expand=1 seems to be all https://canonical.leankit.com/Boards/View/101652562/116603811 needs.
[11:27] <dooferlad> just let me know about printing/logging the deprecation warning and I will tidy up that comment and get it in for review
[11:27] <dimitern> dooferlad, ok, I'll have a look shortly
[11:27] <dooferlad> dimitern: thanks.
[11:28] <dimitern> voidspace, get some lunch first
[11:28] <dimitern> voidspace, you're eating tests otherwise :)
[11:29] <voidspace> :-)
[11:31] <voidspace> dimitern: well, I want to meditate and it will be a little bit of time before I can persuade my wife to clear out of the room I need :-D
[11:31] <dimitern> voidspace, I see ok :)
[11:34] <dimitern> voidspace, dooferlad, only now I realize ListSpaces API doesn't conform to the usual (results, error) behavior - it returns results and an error, but only the first error when fetching a space's subnets
[11:35] <voidspace> dimitern: you mean it should collect all errors
[11:35] <dimitern> it needs to use a Results and Error fields of the results for such errors
[11:35] <voidspace> dimitern: as the only possible error is connection with state failing (so all should fail or none)
[11:35] <voidspace> dimitern: I don't think it's a big issue
[11:35] <dimitern> voidspace, yeah, only return an error if it can't return anything
[11:36] <voidspace> I mean, we should fix it - but add a card for after mvp maybe
[11:36] <dimitern> voidspace, well, just because we're catching this now means it lacks bulk call coverage
[11:36] <voidspace> "bulk call coverage"?
[11:36] <voidspace> it's a bulk response not a bulk call...
[11:37] <voidspace> it's a single call with a bulk result
[11:37] <dimitern> voidspace, yeah - a call to it with e.g. one valid, one invalid, and another valid - checking 2 results and 1 error is returned
[11:37] <dimitern> voidspace, the apiserver method I mean
[11:37] <voidspace> right
[11:38] <dimitern> voidspace, it's trivial to fix and we should do it for the mvp by all means, as changing the api later is worse
[11:39] <voidspace> dimitern: ok
[11:39] <dimitern> voidspace, I'd add a card for it, but please add a TODO about it in your PR - apiserver/spaces
[11:40] <voidspace> dimitern: TODO added
[11:41] <voidspace> well, pushing now
[11:42] <dimitern> voidspace, thanks!
[11:42] <voidspace> done
[11:44] <voidspace> right, room free
[11:44] <voidspace> off to meditate
[11:46] <dimitern> dooferlad, hey
[11:47] <dimitern> dooferlad, the help docs need to mention the spaces constraints but not --networks btw
[11:49] <dimitern> dooferlad, but there are also - tests we do ignore --networks and dropping those that don't (or at least adding TODOs/skipping them)
[11:50] <dimitern> dooferlad, hmm sorry, I might have described it badly in the card
[11:51] <dimitern> dooferlad, the idea is not only to give a warning for it, but also not to pass it through the layers as well, so it can't interfere with the other work around spaces for deployments
[11:51] <dimitern> dooferlad, does that make sense?
[12:11] <dimitern> does anyone else get build errors for gwacl on master after updating and running godeps ?
[12:12] <dimitern> axw, ping
[12:40] <dooferlad> dimitern: yea, that isn't what I got from the card. I read it as we would still accept --networks, but we should mark it as deprecated to discourage its use before removing it in a later version.
[12:42] <dimitern> dooferlad, we would've done that if currently it was at least remotely useful
[12:43] <dimitern> dooferlad, and it will soon go away to be replaced by deploy time bindings of the right kind, with spaces (once we figure out how'd they look like)
[13:02] <dimitern> anyone up for a trivial review? http://reviews.vapour.ws/r/2377/
[13:05] <dimitern> TheMue, voidspace, dooferlad ^^
[13:07] <dooferlad> dimitern: *click*
[13:24] <dooferlad> dimitern: you said that space contraints are handled elsewhere in the card I am looking at. Where is that?
[13:25] <dooferlad> dimitern: also, with network constraints, do you want me to just remove them or just not wire them up in the CLI?
[13:25] <dooferlad> or just leave as-is I guess.
[13:26] <dimitern> dooferlad, they need to go, but that's a separate card, which will require a lot of prerequisites (mostly provider and provisioner related) landed first
[13:27] <dimitern> dooferlad, not sure what do you mean by "handled" - my PR implements parsing and tests for them, and since they're constraints that makes them usable everywhere
[13:27] <dooferlad> dimitern: doh! Haven't merged that in :-)
[13:28] <dimitern> whether we do anything with them is another thing
[13:34] <dimitern> dooferlad, did that answer your questions btw? :)
[13:35] <dooferlad> dimitern: I am leaving the constraints code as it is, so it will accept networks. I have removed the code path that --networks used for juju deploy.
[13:35] <dooferlad> dimitern: I have had a look at add-machine and add-unit and both seem to have no explicit --network support, so they are just constraints code again
[13:35] <dooferlad> dimitern: if that is right, yes.
[13:37] <dimitern> dooferlad, indeed - I just double checked
[13:37] <dimitern> dooferlad, are there a lot of tests affected by --networks being ignored ?
[13:38] <dooferlad> dimitern: I just removed the two that started to fail (correctly) once support for the --network flag was removed.
[13:38] <dooferlad> dimitern: I didn't look at any already skipped ones.
[13:40] <dimitern> dooferlad, by "removed" do you mean it returns an error like "flag provided but not defined: --networks" or it accepts it, prints a warning and continues?
[13:40] <dooferlad> prints a warning and continues as if it isn't there
[13:40] <dimitern> dooferlad, great!
[13:40] <dooferlad> dimitern: http://reviews.vapour.ws/r/2382/
[13:41] <dooferlad> dimitern: I haven't added any support, so much as removed other support.
[13:41] <dimitern> dooferlad, that's a lot better - less fallout than my original attempt which did the former
[13:41] <dooferlad> so the title is rubbish
[13:49] <dimitern> dooferlad, it's fine :)
[13:49] <dimitern> dooferlad, almost done with the review btw
[14:05] <dimitern> dooferlad, was the removal of that status test case necessary?
[14:09] <dooferlad> dimitern: yes, it failed.
[14:14] <dimitern> dooferlad, that's because we're not stopping tests from calling deploy APIs with networks
[14:15] <dimitern> dooferlad, rather than removing it, can you comment it out for now with a TODO about fixing it once the API and state layers also ignore (requested)networks (which --networks sets effectively) ?
[14:15] <dimitern> dooferlad, I'll add a comment to the review
[14:21] <dimitern> dooferlad, reviewed
[14:43] <voidspace> dimitern: ping
[14:47] <dimitern> voidspace, pong
[14:47] <voidspace> dimitern: so, I add a *Error field to params.Space so we can return partial results
[14:48] <voidspace> dimitern: the top level params.ListSpaceResult has an Error field (in case fetching the spaces fails - the new error is for an error fetching subnets for an individual space)
[14:48] <voidspace> dimitern: and ListSpaces also returns an error
[14:49] <voidspace> dimitern: so if we get an error fetching the subnets for a single space obviously we set the Error field on Space
[14:49] <dimitern> voidspace, you did all this in your subnets api branch?
[14:49] <voidspace> dimitern: do we set an error on ListSpaceResult and do we return an error too?
[14:49] <dimitern> voidspace, or in a follow-up?
[14:49] <voidspace> dimitern: no, new branch - looking at it now
[14:49] <dimitern> voidspace, ah, great
[14:49] <voidspace> dimitern: not sure if I can get it done before I go as I go to the gym in 40minutes
[14:49] <voidspace> dimitern: that branch landed on net-cli by the way
[14:49] <dimitern> voidspace, did the other one land?
[14:50] <voidspace> yep
[14:50] <dimitern> voidspace, awesome!
[14:50] <dimitern> voidspace, so I added a card for the ListSpace/Subnet errors
[14:50] <voidspace> I tried looking for other examples in the apiserver but failed to find any and thought it would be quicker to ask
[14:50] <voidspace> dimitern: it should just be ListSpace I believe
[14:50] <voidspace> dimitern: ListSubnets doesn't make extra calls per subnet, so either it all works or it all fails
[14:50] <dimitern> voidspace, so usually bulk calls that return many results for many entities
[14:50] <voidspace> (I believe)
[14:51] <voidspace> dimitern: with ListSpaces we can fetch the spaces and then fail to fetch the subnets for some of the spaces
[14:51] <voidspace> so partial results make sense in theory
[14:51] <dimitern> voidspace, ..only return errors if it's not possible to do its job
[14:51] <voidspace> I say in theory, because in practise the only error is that the connection to state goes
[14:51] <voidspace> so again, in reality it will be all or nothing
[14:51] <voidspace> but it's better to stick to the pattern
[14:52] <voidspace> dimitern: right, so we don't return a top level error
[14:52] <voidspace> dimitern: what about setting an error on ListSpacesResult?
[14:52] <dimitern> voidspace, let me have a look at the code now for a moment
[14:52] <voidspace> dimitern: or should we rely on the client checking the Error field of each Space
[14:55] <dimitern> voidspace, for ListSpaces you're correct - the backing error should be returned as top-level, others can only be per-space (fetching subnets failed)
[14:56] <cmars> wallyworld, http://reviews.vapour.ws/r/2387/ please
[14:57] <voidspace> dimitern: I'm sorry I don't understand
[14:57] <voidspace> dimitern: what is "the backing error"?
[14:57] <voidspace> dimitern: if we get an error fetching the subnets for a space obviously we set the Error on the params.Space
[14:57] <dimitern> voidspace, coming from the state (backing)
[14:57] <voidspace> dimitern: do we set an error anywhere else?
[14:58] <voidspace> dimitern: so we also set an error on ListSpaceResults? or are you saying we also return the error at the very top level?
[14:59] <dimitern> voidspace, sorry, let me explain better
[14:59] <dimitern> voidspace, https://github.com/juju/juju/pull/2980/files#diff-ec0483c931b7d3d2d378df66d465deb1R100 < this is a top-level error (with empty results)
[15:00] <voidspace> dimitern: yes
[15:00] <dimitern> voidspace, https://github.com/juju/juju/pull/2980/files#diff-ec0483c931b7d3d2d378df66d465deb1L109 < this is a per result error
[15:00] <voidspace> yes
[15:00] <voidspace> dimitern: that's what we do currently
[15:00] <dimitern> voidspace, in the latter case, the results != empty, err = nil
[15:00] <voidspace> right
[15:01] <voidspace> and we set the error on the individual params.Space we create
[15:01] <voidspace> (we should I mean)
[15:01] <voidspace> I'm asking do we *also* do anything else with it?
[15:01] <dimitern> voidspace, there's no individual params.Space
[15:01] <dimitern> voidspace, ListSpaces takes no args at apiserver side
[15:02] <voidspace> the object there "result" is a params.Space
[15:02] <voidspace> so instead of "return results, err" it would be
[15:03] <voidspace> result.Error = common.ServerError(err); result.Results = append(result.Results, result);continure
[15:03] <voidspace> *continue
[15:03] <dimitern> voidspace, ah, got you
[15:03] <voidspace> dimitern: that's the change you have said we ought to make
[15:03] <dimitern> voidspace, yes
[15:03] <voidspace> I'm asking if we should *also* set results.Error
[15:03] <dimitern> voidspace, and I've just realized ListSubnets apiserver side is not yet done, so it's fine
[15:03] <voidspace> and we've agreed that the returned err should still be nil
[15:04] <voidspace> ListSubnets won't have the same problem
[15:04] <voidspace> I'm asking if we should *also* set results.Error
[15:04] <voidspace> and we've agreed that the returned err should still be nil
[15:04] <dimitern> voidspace, yes, if you can fetch spaces ok, but not all subnets -> return results(with Error set as needed), nil
[15:05] <dimitern> voidspace, if you can't fetch spaces - as it is now
[15:05] <voidspace> so I should set result.Error for each Space where fetching subnets fail, and set a results.Error to indicate that there are only partial results
[15:06] <dimitern> voidspace, sorry, I'm confused - what do you mean by result.Error for each space?
[15:06] <voidspace> dimitern: every space we fetch is represented by a params.Space
[15:06] <dimitern> voidspace, yes, yes
[15:06] <voidspace> dimitern: that will grow an Error field
[15:06] <dimitern> voidspace, sorry
[15:06] <voidspace> dimitern: that is settled
[15:06] <dimitern> voidspace, I need to get in a call
[15:06] <dimitern> voidspace, it is, yes
[15:06] <voidspace> dimitern: I am asking, do we *also* set an Error on ListSpaceResults
[15:07] <dimitern> voidspace, where is that ListSpaceResults defined?
[15:08] <voidspace> sorry params.ListSpacesResults
[15:08] <voidspace> at the moment it doesn't have an Error field because the method would either return all results or an error
[15:08] <voidspace> so it didn't need one there were no partial results
[15:08] <voidspace> we're now adding partial results
[15:09] <voidspace> so I wonder if ListSpacesResults (apiserver/params/network.go I believe)
[15:09] <voidspace> I wonder if it now also needs an Error field
[15:09] <voidspace> used to indicate partial results
[15:09] <dimitern> voidspace, I believe we need to make api.ListSpaces() ([]params.Space, error)
[15:10] <voidspace> dimitern: earlier today you said that apiserver couldn't return []params.Space and ListSpacesResults was still needed :-)
[15:11] <dimitern> voidspace, then also change ListSpacesResults' Results field to be of type ListSpacesResult (or whatever), which has a Space and Error *Error fields
[15:12] <voidspace> instead of adding an Error to Space...
[15:12] <voidspace> add a new intermediate type
[15:12] <dimitern> voidspace, it's just confusing because of the api/apiserver transition
[15:12] <voidspace> I think you're still slightly misundestanding my question/suggestion :-(
[15:12] <voidspace> I think you have too much in your head
[15:12] <voidspace> we don't need a new intermediate result type, we can just add an Error field to Space
[15:13] <dimitern> voidspace, ok, I see how params.Space can be used as what I'm describing as ListSpacesResult (singular)
[15:13] <voidspace> I am asking if the *plural*, the container results type, ListSpacesResults also needs an Error field
[15:13] <voidspace> because if we get partial results (an error occurred somewhere) don't we want a top level Error field to indicate that the results are partial?
[15:14] <voidspace> maybe not, and just having the error printed inline with the space description is fine
[15:15] <dimitern> voidspace, it's not common to have ErrPartialResults from API server methods
[15:15] <dimitern> voidspace, and in this case I don't think it's even useful
[15:15] <voidspace> ok
[15:15] <voidspace> so we won't
[15:15] <voidspace> thanks
[15:15] <dimitern> voidspace, thanks for bearing with me :)
[15:15] <voidspace> :-)
[15:16] <voidspace> I go soon
[15:16] <voidspace> so I'll be leaving you to it
[15:17] <natefinch> ericsnow: if we always flush immediately after set, we should be able to remove the mergeProcMaps
[15:17] <dimitern> voidspace, sure
[15:18] <ericsnow> natefinch: right
[15:21] <voidspace> dimitern: cmd/juju/spaces/list.go - ListSpaces command fetches the subnets using api.AllSubnets()
[15:21] <voidspace> dimitern: it doesn't use the subnets on the Space!!
[15:21] <voidspace> I guess space.Subnets() is newer than the ListSpaces command...
[15:22] <voidspace> I don't think I can fix that in the time I have
[15:22] <voidspace> it will make the code simpler though
[15:22] <voidspace> I'll add it to the card
[15:23] <dimitern> voidspace, ack, otp - thanks
[15:26] <voidspace> dimitern: done, card updated - including the code to set the error correctly
[15:26] <voidspace> dimitern: have a good week
[15:26] <voidspace> TheMue: dooferlad: EOW
[15:26] <voidspace> I begin my journey home
[15:26] <voidspace> good luck with the MVP
[15:26] <TheMue> voidspace: have a safe trip
[15:26] <voidspace> thanks
[15:28] <natefinch> ericsnow: updated the code according to your suggestions, would like a re-review so we can get it done
[15:28] <TheMue> dimitern: maybe I'm missing something and you can point me to the right way. I've got my test, I've got my mock, but the way to inject is seems difficult
[15:28] <ericsnow> natefinch: will do
[15:47] <natefinch> ericsnow: realized I didn't publish my responses to your earlier comments.  Just did so.
[15:47] <ericsnow> natefinch: thanks
[15:47] <natefinch> (only one real comment)
[16:01] <dimitern> TheMue, can I look at the code you're having issues with?
[16:03] <dooferlad> dimitern: do you have an opinion about what card I should do next?
[16:05] <dimitern> dooferlad, I don't see a card for apiserver subnets - create and lest
[16:05] <dimitern> list
[16:05] <TheMue> dimitern: it's not a concrete issue, more a missing idea.
[16:06] <dooferlad> dimitern: you meen the apiserver side of what voidspace has done?
[16:06] <TheMue> dimitern: in the test in http://paste.ubuntu.com/12109009/ I try to inject the mock
[16:06] <dimitern> dooferlad, yeah
[16:07] <dimitern> dooferlad, we seem to have missed this - can you add a card and start on Add first perhaps?
[16:07] <dimitern> TheMue, I'll have a look shortly
[16:07] <dooferlad> dimitern: isn't it in the archive? https://canonical.leankit.com/Boards/View/101652562/115620070
[16:07] <dooferlad> dimitern: also https://canonical.leankit.com/Boards/View/101652562/115620186
[16:08] <dimitern> dooferlad, both of these are for spaces
[16:08] <dooferlad> dimitern: doh!
[16:08] <dooferlad> dimitern: yes, you are right
[16:09] <dimitern> dooferlad, I know it's confusing - it gets me every time :)
[16:12] <dimitern> dooferlad, and I'm wrong - apiserver has AddSubnets, but no list yet
[16:12] <dimitern> dooferlad, we don't need CreateSubnets for the MVP, so we can just do Add and List
[16:13] <dimitern> dooferlad, there's a card I added re ListSpacesResults and ListSubnetsResults - if you prefer
[16:13] <dimitern> dooferlad, also voidspace has a branch with some progress on it
[16:15] <dooferlad> dimitern: https://github.com/voidspace/juju/commits/spaces-subnets-api ?
[16:16] <dooferlad> dimitern: that is merged
[16:17] <dimitern> dooferlad, I think this is it - https://github.com/voidspace/juju/tree/spaces-list-errors
[16:18] <dooferlad> dimitern: that is about spaces, not subnets
[16:18] <dooferlad> but if both need list, then I can probably finish that and add subnets
[16:18] <dimitern> TheMue, it's about ListSpaces returning proper results, which will apply to ListSubnets as well even more (apiserver side)
[16:19] <dimitern> dooferlad, ^^
[16:19] <dimitern> TheMue, sorry, I meant to tell you to have a look at TestManageEnvironDoesNotRunFirewallerWhenModeIsNone
[16:19] <TheMue> dimitern: ah, thx. what a java-like name
[16:20] <dimitern> TheMue, :)
[16:20] <dimitern> TheMue, the next one after it is another example
[16:20] <dimitern> TheMue, since this is a pre-existing test suite, no need to go wild with mocks and things when they're not already in place
[16:21] <dimitern> dooferlad, does that make sense?
[16:21] <dooferlad> dimitern: yes
[16:22] <dimitern> dooferlad, cheers, let's discuss it tomorrow if needed
[16:22] <dooferlad> dimitern: it is EOD for me, so will make some notes in that card about what is needed and start tomorrow, at which point I will probably have questions :-)
[16:23] <TheMue> dimitern: my test so far is pretty similar, but environs.SupportsNetworking() tests against the environment. that's why I want to mock it here
[16:23] <dimitern> dooferlad, awesome!
[16:23] <dimitern> TheMue, you don't need to mock it with the dummy provider
[16:23] <TheMue> dimitern: when using dummy environs.SupportsNetworking() always returns true
[16:27] <dimitern> TheMue, ah, yeah - it wasn't calling SupportsAddressAllocation
[16:27] <dimitern> TheMue, then, the mocks are unfortunately needed
[16:27] <dimitern> TheMue, *or*
[16:28] <dimitern> TheMue, I guess you can make it easier to write tests  for all future cases like this
[16:28] <dimitern> TheMue, by making the dummy provider *not* support networking when a given environ config setting is there
[16:30] <TheMue> dimitern: yeah, the better approach, as we already discussed about, is mnot to use type assertion
[16:31] <dimitern> TheMue, yeah - it will make writing tests a lot easier
[16:32] <TheMue> dimitern: definitely
[16:42] <TheMue> dimitern: I change the card description with my ideas. could you take a look please?
[16:45] <dimitern> TheMue, looks good, only the name SupportsNetworking() on netEnv needs to be better I think
[16:47] <dimitern> TheMue, but I need to think about it - I'd suggest to go ahead and do it as you suggest, we can rename the NetworkingEnviron method name later as you propose it
[16:49] <TheMue> dimitern: exactly, renaming it en bloc is simple
[16:49] <dimitern> TheMue, +1
[17:00] <TheMue> dimitern: while doing it, how about ProvidesAddressAllocation or AllowsAddressAllocation for Allocate... and Release...
[17:00] <TheMue> dimitern: ?
[17:27] <dimitern> TheMue, we have this already - SupportsAddressAllocation - true for both Release and Allocate
[17:27] <dimitern> anyway, EOD for me
[17:28] <TheMue> dimitern: enjoy
[17:28] <dimitern> TheMue, thanks, same to you :)
[17:28] <TheMue> dimitern: ta
[19:33] <katco> natefinch: hey are you still blocked on feature tests for juju status?
[19:35] <natefinch> katco: I haven't spent time on it, but in theory, no.
[19:35] <natefinch> katco: I can unblock the card
[19:35] <katco> natefinch: k just checking. probably makes sense to stay on the 3-pointer?
[19:36] <natefinch> katco: yeah
[19:36] <katco> natefinch: kk
[19:39] <katco> wwitzel3: were you working on the rename cards?
[19:40] <wwitzel3> katco: haven't started them yet, just about have the feature test buttoned up, then that was my next item to start
[19:40] <katco> wwitzel3: cool
[19:41] <wwitzel3> katco: thanks to ericsnow's hand holding, I am just about done with the test
[19:41] <katco> lol
[20:46] <natefinch> ericsnow: does parseID return name, id or id, name? https://github.com/juju/juju/blob/feature-proc-mgmt/process/info.go#L42
[20:48] <natefinch> ericsnow: nvm, usage indicates name, id
[20:48] <ericsnow> natefinch: k
[22:06] <mup> Bug #1485781 opened: Juju is unreliable on Joyent <joyent-provider> <reliability> <repeatability> <juju-core:Triaged> <https://launchpad.net/bugs/1485781>
[22:33] <mup> Bug #1485784 opened: Error creating container juju-trusty-lxc-template; Failed to parse config <oil> <juju-core:New> <lxc:New> <https://launchpad.net/bugs/1485784>
[23:27] <davecheney> thumper: free now ?
[23:28] <thumper> davecheney: just munching down some food, 5 min?
[23:28] <davecheney> i'll see you in the hangout
[23:28] <davecheney> take your time