19:03:28 <dtroyer> #startmeeting OpenStackClient
19:03:29 <openstack> Meeting started Thu Dec 17 19:03:28 2015 UTC and is due to finish in 60 minutes.  The chair is dtroyer. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:03:30 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
19:03:32 <openstack> The meeting name has been set to 'openstackclient'
19:03:34 <dtroyer> anyone here for the OSc meeting?
19:04:10 <stevemar> dtroyer: oh you're alive
19:04:20 <dtroyer> courtesy ping:briancurtin, terrylhowe, lhcheng, dstanek, MeganR
19:04:28 <dtroyer> stevemar: you only think that because you can't see me
19:04:29 <stevemar> i was just talking to terrylhowe that i was going to ditch this weeks installment
19:04:45 <terrylhowe> o/
19:04:50 <lhcheng> o/
19:04:53 <dtroyer> I'm planning to cancel the next two weeks
19:04:57 <stevemar> but it we actually have quorum...
19:05:00 <dtroyer> like most end-of-week meetings
19:05:16 <rtheis> o/
19:05:35 <stevemar> hey rtheis!
19:05:35 <dtroyer> ok, let's get started
19:05:44 <dtroyer> #topic reviews
19:05:49 <rtheis> hi stevemar
19:06:12 <dtroyer> I have a couple of questions I've raised in reviews that we might want to talk about here to get a wider audience
19:06:23 <dtroyer> and then go from there
19:07:00 <stevemar> sure, where do we start?
19:07:04 <dtroyer> https://review.openstack.org/#/c/257403/
19:07:28 <dtroyer> This question boils down to how we handle errors in commands that make multiple REST API calls
19:08:04 <dtroyer> Niall has a nice set of tables in one of his later comments that shows the tradeoff
19:08:58 <dtroyer> assumption #1: we don't do rollbacks, so if there are two API calls and the first succeeds and second fails, the changes for the first stay
19:09:11 <dtroyer> (Think of the set commands for context)
19:09:27 <dtroyer> if the first fails, should we attempt the second?
19:10:00 <dtroyer> Anyone have an idea what a user expectation might be here?
19:11:14 <stevemar> what are the other options?
19:11:15 <terrylhowe> I like the idea of try all the steps, print errors for what fails and return 1 for any failure or partial fail
19:11:30 <lhcheng> I guess it depends if the second command makes sense..
19:11:30 <dtroyer> Niall's patch has abort after first failure, don't attempt the rest
19:11:49 <dtroyer> lhcheng: yes, there are probably cases where it doesn't make sense
19:11:57 <dtroyer> this one may not be one of those
19:12:07 <dtroyer> order is not important here
19:12:20 <terrylhowe> from the users perspective, they might not know that if the update failed, the activate worked
19:13:19 <stevemar> its gonna depend on the command, for multi-deletes we should report back all fails and keep going. but for something that is like saving an image, if the GET fails, don't proceed
19:13:54 <dtroyer> right stevemar, I'm thinking multiple calls for one OSC command, not looking over a list
19:14:03 <dtroyer> looping over a list
19:16:18 <dtroyer> anyone with a strong opinion add it to the review, I can live with either although my gut says we should do as much as we can
19:17:20 <stevemar> dtroyer: i like the patch
19:17:34 <terrylhowe> just like the delete with nargs, it is easier to track if we print errors and try everything
19:18:18 <dtroyer> terrylhowe: it is consistency with that mode that I like
19:19:19 <dtroyer> moving on
19:19:21 <dtroyer> https://review.openstack.org/#/c/246459/
19:19:29 <lhcheng> consistency is basic for better user exp, I vote for consistency of behavior.
19:19:56 <dtroyer> This adds —project-domain to image delete.
19:20:09 <dtroyer> stevemar: asked about aliasing —project for —owner.
19:20:33 <dtroyer> I like that idea, I suppose the point is that we should do this review and the alias one in the same release
19:21:12 <lhcheng> sounds good
19:21:49 <lhcheng> makes our arg naming consistent
19:22:02 <dtroyer> so if we think we're close to a release I'd like to wait on this.  I can do the follow-up, but probably not this week
19:22:05 <stevemar> sounds good, we can switch all instances of owner to project and make an alias for owner
19:22:18 <stevemar> dtroyer: no need to release til new year
19:22:29 <dtroyer> ok, cool.
19:24:14 <dtroyer> last on my list is https://review.openstack.org/#/c/251646/
19:24:38 <dtroyer> and this is basically about if set commands should have any output similar to create commands
19:25:24 <dtroyer> this particular command used List for a specific reason, most that do have output should be using Show.
19:25:46 <dtroyer> the question is are there any set commands that create something new that needs to be reported to the user?
19:25:57 <dtroyer> if not, I am OK with removing output from set commands
19:26:03 <dtroyer> I can't hink of any
19:26:42 <stevemar> we'd have to go through and see what they're doing
19:26:52 <stevemar> i assume it's just showing the newly updated object
19:26:54 <stevemar> resource
19:27:22 <dtroyer> right.  the create command returns a new ID, set doesn't do that AFAICT
19:27:28 <dtroyer> with other fields
19:28:04 <stevemar> no, most of the time set -> PATCH/PUT which is a 204 usually
19:28:11 <stevemar> and doesn't return any content
19:28:27 <stevemar> at least, that's how it is in keystone
19:29:06 <dtroyer> I'm not sure the REST semantics matter here
19:29:15 <stevemar> hmm, i'm wrong
19:29:22 <stevemar> we do return the new object and it's 200 OK
19:29:27 <dtroyer> it's what the API is actually doing
19:30:14 <dtroyer> the point is to make doing an immediate show command after a set unnecessary for the user to get some new bit of information
19:30:45 <stevemar> another way of looking at it: we'd probably make a lot of people happy by showing the new object, whereas we'd probably upset folks if we removed capability
19:31:20 <dtroyer> right, and I don't want to have to revisit a decision to remove something in six months ;)
19:31:25 <terrylhowe> for the services that 204, that’d be expensive
19:31:47 <terrylhowe> I think return nothing is best for consistency
19:32:19 <terrylhowe> anythng that is making a resource under the covers and returning an id, i probably is handled as a ‘add/remove’ anyway
19:32:38 <dtroyer> stevemar put a list of commands using ShowOne into a comment, it is surprisingly short, so mayybe it isn't too bad…
19:33:09 <dtroyer> and looks like it is mostly Compute API?
19:34:00 <stevemar> i was pro-returning nothing
19:34:07 <dtroyer> so again, lets get opinions into the review.
19:34:17 <stevemar> but now i feel like returning the new object might be better long term
19:35:39 <dtroyer> also, for both this and the multi-call review, whatever we decide we need to add to our developer docs
19:36:06 <stevemar> yep
19:36:18 <dtroyer> that's my list, anyone have any other reviews they want to bring up?
19:36:38 <lhcheng> I got one
19:36:40 <stevemar> regarding release, here's a list of changes that are committed but not released: http://docs-draft.openstack.org/03/257403/5/check/gate-python-openstackclient-docs/5ec748a//doc/build/html/history.html
19:37:01 <stevemar> we last released on 2015-12-03
19:37:28 <terrylhowe> wow
19:37:30 <stevemar> theres a lot of networky goodness going in
19:37:36 <dtroyer> we've addded a bunch of network stuff, is that at a clean breakpoint and working?
19:37:48 <stevemar> dtroyer: that's the thing
19:38:02 <terrylhowe> we don’t have full router crud
19:38:07 <stevemar> i think it's reasonable to wait til new year and get a bit more network stuff under our belt
19:38:23 <stevemar> wait til we get full router and subnet crud
19:38:52 <dtroyer> ok.  let's keep in mind then to plan some checkpoints so the new stuff is complete in logical sections
19:39:24 <stevemar> dtroyer: we should also advocate for more release notes in patches that fix bugs :)
19:39:26 <dtroyer> a lot of those changes were small and things that I'd have grouped together a bit more
19:39:47 <stevemar> a lot are tests and refactoring
19:39:49 <dtroyer> yes, I didn't want to hammer in in-flight bits yet, but probably start making the commnets now
19:40:03 <dtroyer> we have examples which will help
19:40:46 <dtroyer> I'd prefer to keep the test/refactor bits out of the user release notes, they really don't care
19:41:20 <dtroyer> oh, that's not the list I thought it was
19:41:22 <dtroyer> nevermind
19:41:43 <stevemar> dtroyer: that's the "changes" list
19:41:52 <stevemar> lhcheng: you had a topic
19:42:03 <rtheis> I have one review to bring up after lhcheng
19:42:21 <stevemar> rtheis: cool cool
19:43:04 <lhcheng> about tests, there's a couple of patches that added test for private methods.
19:43:15 <lhcheng> I feel like it is over-testing
19:43:47 <lhcheng> It is harder to refactor later if we also have test for private methods
19:43:51 <lhcheng> https://review.openstack.org/#/c/254142/
19:44:37 <stevemar> i feel like for this private method in particular it's OK
19:44:44 <stevemar> since it's a formatter
19:44:48 <stevemar> and a simple on
19:44:49 <stevemar> one
19:44:59 <stevemar> i agree its probably overkill for most
19:45:17 <stevemar> this one has a very narrow scope
19:45:57 <terrylhowe> valuable test I think, way easier to have a unit test for that kind of thing
19:46:29 <terrylhowe> I agree no tests for privates in general, but this looks like a good exception to that rule
19:46:54 <dtroyer> so I hear this is a discretionary thing not a hard rule,but the default is what terrylhowe just said above?
19:47:14 <lhcheng> okay, just didn't want overtest and write test for all private method
19:47:25 <terrylhowe> agreed
19:47:55 <lhcheng> dtroyer: ok, sounds good to me
19:48:06 <dtroyer> lhcheng: thanks for raising the thought, I don't think we've ever had a 'too much testing' problem in OSC ;)
19:48:36 <dtroyer> with the rate that tangchen is writing them though…
19:49:09 <lhcheng> dtroyer: hah
19:49:19 <dtroyer> rtheis: you had a topic?
19:49:27 <rtheis> yes, thank you: https://review.openstack.org/#/c/257543/
19:49:43 <rtheis> I haven't found a great way to move existing networking commands (such as security groups) to sdk, and would appreciate ideas on the best way forward.
19:49:53 <stevemar> lhcheng: if another patch comes up for testing private methods we can pile on the -2's
19:49:53 <stevemar> hehe
19:49:53 <stevemar> i'd hate to see a coverage report for osc :(
19:50:27 <dtroyer> I've only glanced at this but it seems like a decent model for commands with multiple back-end implementations
19:51:05 <dtroyer> also, I think the compute/nova-net bits may be the only ones where we need to support the proxied API so the major place it might come up again is in plugins
19:51:06 <rtheis> I tried playing with command loading but didn't get very far
19:51:48 <terrylhowe> I’m concerned how this will look on more ocmplicated examples where there are different parseargs
19:52:22 <terrylhowe> we might have to figure this stuff out at command load time rather than run time
19:52:25 <rtheis> terrylhowe: yeah this model pins the commands to the same arguments which we may not always want
19:53:16 <rtheis> For example, security group rules in nova hide ethertype and direction but neutron exposes them
19:53:58 <dtroyer> can we accept them as optional on the nova version and ignore them?  not sure of the message that sends to users though
19:54:21 <dtroyer> but like some of the policy things we have, in help we would need to do at least an auth pass to get the SC to even know which version to show
19:57:58 <rtheis> yeah, maybe well need to do an auth pass as part of help for these.  I'll take a look at that and possibly ignoring optional parameters
19:58:05 <rtheis> we'll
19:58:25 <terrylhowe> if neutron loaded last here? https://github.com/openstack/python-openstackclient/blob/master/openstackclient/common/commandmanager.py#L55
19:59:00 <terrylhowe> no that isn’t right
19:59:07 <dtroyer> terrylhowe: even if it did, that doesn't account for plugins
19:59:23 <dtroyer> we can't make assumptions about load order
19:59:27 <terrylhowe> maybe we should load neutron like a plugin
19:59:36 <dtroyer> it already is, it is just in-repo
19:59:54 <terrylhowe> well, if it could be deterministic some how
20:01:29 <terrylhowe> https://github.com/openstack/python-openstackclient/blob/master/openstackclient/shell.py#L314
20:01:47 <dtroyer> rtheis: I've though having a wrapper command in openstackclient.comm that handles the switch much like your new class does would be useful, and have that call out to the individual commands
20:01:49 <terrylhowe> nova there and neutron after that
20:02:22 <rtheis> dtroyer: thanks, I'll take a look at that.
20:02:40 <dtroyer> terrylhowe: one reason why I think the switch decision needs to be as late as it can
20:03:11 <dtroyer> rtheis: I never got all the way thinking that through so it may be a dead end, but having another mind on it would be great
20:03:27 <rtheis> sure
20:03:58 <dtroyer> ok, we're over time
20:04:11 <dtroyer> no meetings the next two weeks, see everyone on Jan 7!
20:04:22 <dtroyer> #endmeeting