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