Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2018-10-09
15:28:52 smcginnis dtroyer: This is for the next command I had started to work on, so different than the one you had commented on.
15:35:51 openstackgerrit Dmitry Tantsur proposed openstack/openstacksdk master: Wire in retries for all baremetal actions https://review.openstack.org/603427
15:41:54 dtroyer smcginnis: I didn't think anything special needed to be set up, we're already using it in the OSC repo.
15:42:21 dtroyer I'm looking to see if there is any additional config required...
15:42:48 smcginnis dtroyer: Do you know how it maps the command name to the class name? Maybe I have something messed up there.
15:43:13 dtroyer it's part of cliff so it uses the entrypoint mapping
15:52:09 smcginnis dtroyer: Got it. I needed a "*" at the end to pick up the actual command. I had "volume capability" but needed "volume capability *".
15:56:37 smcginnis dtroyer: I'm realizing both cinder list-pools and get-capabilities are really backend-centric commands and "volume capability show" doesn't make sense. Do you think "volume backend capabilities" and "volume backend pools" make sense? Or should it be the long "volume backend capability show" and "volume backend pool list" form be preferrable?
15:57:06 smcginnis Personally I like the shorter, but having "list" and "show" seems to be the more commonly used formats.
16:00:04 dtroyer smcginnis: this is the hardest part, naming things. 'backend' by itself is still too generic so it needs the qualifier, if something other than 'volume' makes sense that would be fine, but I can't think of one, 'volume backend' seems correct for what you are describing. but ya, it is really long
16:00:55 dtroyer is 'storage capabilities' or 'storage pools' too generic? we have multiple types of storage so it may be
16:02:13 smcginnis Yeah, I think "volume backend" is used enough that it most likely would make sense to users, wheras "storage foo" might be too generic.
16:03:03 smcginnis What about including the "show" and "list" at the end vs just "volume backend capabilities" and "volume backend pools"?
16:04:00 dtroyer I'll strongly recommend to stick with the 'object verb' command format…it's less to remember which verb to use based on the kind of output you want than to rememebr which commands don't have one
16:21:21 smcginnis dtroyer: OK, so "volume backend capability show" and "volume backend pool list" then?
18:48:38 dtroyer smcginnis: I think so
18:49:53 smcginnis dtroyer: Cool, thanks. That's what I'm working on now.
20:26:49 openstackgerrit Sean McGinnis proposed openstack/python-openstackclient master: Add volume backend capability show command https://review.openstack.org/609122
20:32:06 openstackgerrit Sean McGinnis proposed openstack/python-openstackclient master: Add volume backend capability show command https://review.openstack.org/609122
20:56:40 openstackgerrit Sean McGinnis proposed openstack/python-openstackclient master: Add volume pool list command https://review.openstack.org/608740
20:57:29 openstackgerrit Sean McGinnis proposed openstack/python-openstackclient master: Add volume backend pool list command https://review.openstack.org/608740
#openstack-sdks - 2018-10-10
02:33:03 openstackgerrit Nguyen Van Trung proposed openstack/cliff master: Don't quote {posargs} in tox.ini https://review.openstack.org/609185
10:04:44 mordred ml: that patch looks great!
10:05:28 mordred samueldmq: https://review.openstack.org/#/c/604811 <-- you might find this interesting
10:05:44 ml Thanks for the review!
10:10:15 samueldmq mordred: oh that's using the sdk data model
10:15:32 samueldmq mordred: ml : commented on that. suggested a few improvements
10:15:36 mordred samueldmq: yah - I haven't gotten far enough along to explore that yet - but I've been thinking that we could migrate from the normalize methods to using the sdk data model - and maybe some times have some of the logic be in the sdk proxy methods - I was going to try it with create_image
10:15:40 mordred samueldmq: awesome, thanks!
10:16:15 ml samueldmq: Thanks, will have a look#
10:16:36 samueldmq mordred: we'd still need something like normalzie anyways
10:17:09 samueldmq I know sdk data model already maps some of the attributes, such as enabled -> is_enabled in the data model definition
10:17:27 samueldmq but we'd need to map that against the current shade contract/data model
10:17:32 samueldmq because we don't want to change that right
10:20:17 mordred yes, you're exactly right
10:20:49 mordred although maybe it'll be enough to just update the sdk model to be the same as the normalize contract in shade?
10:21:33 samueldmq if the sdk side is okay updating their contact, it'd be awesome
10:22:28 ml samueldmq: you think it's a good idea to put 14 parameters in the function signature? Feels messy
10:23:12 samueldmq ml: well, see https://github.com/openstack/openstacksdk/blob/master/openstack/cloud/openstackcloud.py#L6751-L6766 for create_server
10:23:25 samueldmq ml: don't necessarily need to put in the signature, see the valid_kwargs annotation
10:23:50 samueldmq ml: I'd probably go with that as you don't do any work on the parameters before passsing on to the server
10:24:35 ml ahh, yeah. @_utils.valid_kwargs sounds good
10:26:12 samueldmq ml: awesome. btw I didn't mean you should because create_server does more params than that
10:26:13 ml also, sadly no, name_or_id doesn't uniquely identify the resource since multiple resources can have the same name
10:26:20 samueldmq but for the reasons I put on the review :)
10:27:26 samueldmq ml: so please make it clear in the docs that filters might need to be used when a name is provided
10:27:51 samueldmq also, document what happens if you provider a name and filters combination that gets you more than one resource
10:28:12 samueldmq when you're callling the update or delete mehtod, for example. does it update'em all? delete them all? throws exception?
10:28:38 samueldmq s/throws/raises , throwing exceptions remembers me of java
10:30:18 ml alright, I'll add :raises [exc]: where necessary
10:30:57 samueldmq ml: nice. thanks for that, it's awesome work
10:31:11 samueldmq I have to go afk for a bit
10:31:26 ml samueldmq: Thanks for the review. Will fix asap.
10:58:12 ml wow, I found more unexpected behavior than I expected. Have to touch the resources once again
12:09:20 mnaser mordred: https://github.com/ansible/ansible/pull/44686
12:09:24 mnaser I’ve reviewed it a while back
12:09:38 mnaser And I think it’s mostly pretty well ready I think
12:34:22 dtantsur hi folks, got a second for https://review.openstack.org/#/c/608674/ ?
12:34:39 dtantsur apparently make baremetal update right from the first attempt is too hard :(
12:58:52 mordred dtantsur: lgtm. Shrews ?
12:59:09 Shrews eh?
13:00:18 Shrews { self._alternate_id() or 'id': self.id } works?
13:00:40 Shrews i did not know you could do that with dict keys
13:01:14 dtantsur I should have used brackets I guess :)
13:01:29 dtantsur it's kind of: key = self._alternative_id() or 'id'
13:01:33 dtantsur { key: self.id }
13:02:45 Shrews yep, just threw me off. +3
13:03:36 mordred Shrews: I learn python from dtantsur all the time
13:03:46 dtantsur :D
13:08:26 openstackgerrit Dmitry Tantsur proposed openstack/openstacksdk master: cloud: rename with deprecation validate_node -> validate_machine https://review.openstack.org/609399
13:15:34 mordred dtantsur: that's going to conflict with my delete-all-the-deprecated-things patch :)
13:15:59 dtantsur mordred: it may :) should I switch off utils.deprecated?
13:17:08 mordred yeah - I think maybe, especially since it's in the shade layer where we never remove old things, let's just rename the method and then do a 'validate_node = validate_machine' after it
13:20:17 dtantsur mordred: can I still use the deprecation library itself?
13:20:22 dtantsur or I can provide a custom warning
13:23:48 mordred dtantsur: I think custom warning - like log.debug in this case - would be best
13:24:25 dtantsur hmm, log.debug? I think in a similar situation you used the warnings module (because it's programmer's, not operator's choice)
13:25:16 mordred dtantsur: oh - well ... that's because in that scenario doing the weird thing is actually a legit valid thing ... we might not support a service and the user might be using sdk to get a handle to the rest service
13:25:30 mordred in this case, the user can totally change the call
13:25:34 mordred although - you make a great point ...
13:25:46 mordred if the user was using ansible, they wouldn't have the ability
13:25:56 mordred becuase the ansible module might be using the old call
13:26:04 dtantsur and btw it probably is
13:26:09 mordred yah
13:26:48 mordred so yeah - maybe warnings.warn is better for this one. stupid backwards compat
13:43:03 mordred Shrews: if you have a sec - https://review.openstack.org/#/c/604521/ - you had some -1 on that earlier
13:44:44 Shrews mordred: doesn't appear you did anything with my comment :-P
13:45:22 jroll fungi: thanks for saying what I was trying to say, much better than I did (graphql thread)
13:46:32 fungi jroll: just trying to help get some clarity. i think you made some other good points
13:46:42 jroll :)
13:47:03 mordred Shrews: oh - I didn't? crap
13:57:48 Shrews mordred: nope. comment was on ps6
13:59:50 openstackgerrit Monty Taylor proposed openstack/openstacksdk master: Remove all the deprecated stuff https://review.openstack.org/605508
13:59:51 openstackgerrit Monty Taylor proposed openstack/openstacksdk master: Make it clear that OpenStackCloud is a mixin https://review.openstack.org/608318
13:59:51 openstackgerrit Monty Taylor proposed openstack/openstacksdk master: Start shifting cloud object-store methods to proxy https://review.openstack.org/608317
14:00:45 openstackgerrit Dmitry Tantsur proposed openstack/openstacksdk master: cloud: rename with deprecation validate_node -> validate_machine https://review.openstack.org/609399
14:00:49 dtantsur mordred: ^^
14:02:36 openstackgerrit Monty Taylor proposed openstack/openstacksdk master: Add support for per-service rate limits https://review.openstack.org/604521
14:02:45 mordred Shrews: sorry about that - thanks
14:03:17 mordred dtantsur: ++

Earlier   Later