| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2020-11-17 | |||
| 17:59:47 | openstackgerrit | Mark Chappell proposed openstack/openstacksdk master: Remove duplicate test_zone_create https://review.opendev.org/763062 | |
| 18:27:13 | openstackgerrit | Stephen Finucane proposed openstack/openstacksdk master: Add update_flavor method https://review.opendev.org/762989 | |
| 18:59:05 | openstackgerrit | Artem Goncharov proposed openstack/openstacksdk master: Add update_flavor method https://review.opendev.org/762989 | |
| 19:03:19 | openstackgerrit | Artem Goncharov proposed openstack/python-openstackclient master: Switch compute aggregate functions to SDK https://review.opendev.org/762134 | |
| 19:05:58 | gtema | stephenfin, are you here? | |
| 20:29:37 | openstackgerrit | Artem Goncharov proposed openstack/openstacksdk master: WIP Add support for deprecated nova hosts https://review.opendev.org/763081 | |
| #openstack-sdks - 2020-11-18 | |||
| 01:22:01 | openstackgerrit | Merged openstack/openstacksdk master: Remove duplicate test_zone_create https://review.opendev.org/763062 | |
| 01:22:03 | openstackgerrit | Merged openstack/python-openstackclient master: trivial: Document removal of support for agents https://review.opendev.org/762825 | |
| 03:01:16 | openstackgerrit | Emilien Macchi proposed openstack/openstacksdk stable/victoria: Don't set list_type to dict for server groups. https://review.opendev.org/763119 | |
| 03:01:31 | openstackgerrit | Emilien Macchi proposed openstack/openstacksdk stable/ussuri: Don't set list_type to dict for server groups. https://review.opendev.org/763120 | |
| 03:01:42 | openstackgerrit | Emilien Macchi proposed openstack/openstacksdk stable/train: Don't set list_type to dict for server groups. https://review.opendev.org/763121 | |
| 09:54:42 | stephenfin | gtema: I was finished for the evening, but I'm here now | |
| 10:01:11 | gtema | oki, no problem | |
| 10:01:47 | gtema | "openstack aggregate cache image __aggregate__ __im1__ __im2__ __im3__" is not cool from my pov | |
| 10:03:19 | stephenfin | Probably best to look at the reply I just left on the review. I'm not recommending this in isolation: there are numerous other examples of this is OSC already | |
| 10:04:11 | gtema | well, from what I see when we support multiple inputs under the resource - it is a named parameter | |
| 10:04:27 | gtema | so for me in this case it is really using a named param and not positional | |
| 10:04:42 | gtema | while positional is used only for i.e. deleting multiple resources | |
| 10:05:37 | stephenfin | See my reply. That's not true with OSC today. There are other non-deletion examples of passing multiple child resources | |
| 10:06:27 | stephenfin | And personally I'd like to expand (in a follow-up, of course) some of the ones that only accept a single child resource to accept multiple child resources, e.g. 'aggregate add host' | |
| 10:07:12 | gtema | for me the "object create _container_ _file1_ _file2_" is example where we do this | |
| 10:07:32 | gtema | since deletion is not fitting as example - it is top resource | |
| 10:07:55 | stephenfin | Yes, that's fair. 'object create' is a valid thing too though, and proves that this can work | |
| 10:08:07 | gtema | I know it can work like that | |
| 10:08:21 | gtema | it's just in my experience this is not a good UX | |
| 10:08:36 | stephenfin | Positional arguments wouldn't make sense for something like 'flavor create'. i.e. you wouldn't do 'flavor create 512 4 0' instead of 'flavor create --ram 512 --vcpu 4 --disk 0' | |
| 10:09:05 | stephenfin | but that's because those additional arguments are attributes of the resource. Also, they're all different meaning it's impossible to parse | |
| 10:09:23 | gtema | exactly, and one step further if in one call you want to create multiple flavors ... | |
| 10:09:35 | gtema | here images are also arguments to the aggregate | |
| 10:09:43 | stephenfin | they're not though | |
| 10:09:48 | stephenfin | they're resources in their own right | |
| 10:10:00 | stephenfin | the command is called 'aggregate cache image' | |
| 10:10:56 | stephenfin | so you're caching an <image> for an <aggregate>; those are two separate resources and there aren't multiple attributes to be set | |
| 10:11:44 | gtema | well yes. When you use named params you can really mix positions of positional and named ones | |
| 10:12:03 | gtema | create image _name_ --param1 --param2 | |
| 10:12:05 | gtema | and | |
| 10:12:13 | gtema | create image --param1 name --param2 | |
| 10:12:38 | gtema | with this way of using only positional ones you have absolutely no choice | |
| 10:13:06 | stephenfin | I don't think this is an issue | |
| 10:13:43 | gtema | :) - for me yes. When you use IDs instead of names you have no clue looking to the command you type whether order is right or not | |
| 10:14:02 | gtema | I was having with that pretty much problems in DNS area | |
| 10:14:03 | stephenfin | traditional would dictate if your command is called '<resource_a> <action> <resource_b>', then the invocation of that looks like '<resource_a> <resource_b>' | |
| 10:14:54 | gtema | "image cache cde42cs abcdef46423 cbfedf454334" - what is what | |
| 10:15:16 | stephenfin | 'server add floating ip' -> 'SERVER FLOATING_IP'; 'aggregate add host' -> 'AGGREGATE HOST' | |
| 10:15:31 | stephenfin | the first one is the aggregate, of course :) | |
| 10:15:38 | gtema | can you add multiple FIPs in same command? | |
| 10:16:12 | stephenfin | no, because the FIPs have an optional attribute | |
| 10:16:20 | stephenfin | (fixed IP address) | |
| 10:16:38 | gtema | aggregate add host also doesn'T allow multiple hosts | |
| 10:16:53 | stephenfin | <stephenfin> And personally I'd like to expand (in a follow-up, of course) some of the ones that only accept a single child resource to accept multiple child resources, e.g. 'aggregate add host' | |
| 10:17:03 | gtema | :D | |
| 10:17:19 | stephenfin | and again, 'object create' does, so we have prior art here | |
| 10:17:28 | gtema | I started SDK change for adding host support and was pulling hears from my head yesterday | |
| 10:18:17 | stephenfin | :-D OSC and openstacksdk are both opinionated libraries. It's fair to expect strong opinions ;) | |
| 10:18:33 | gtema | that's true | |
| 10:18:42 | stephenfin | Also, to my credit, I think the rest of my reviews have been fair and I've caught a good few bugs | |
| 10:19:11 | gtema | let me perhaps say like that: you come from nova, so I expect you more to be responsible for the nova CLIs | |
| 10:20:15 | gtema | to your credit - sure. Great work | |
| 10:20:32 | stephenfin | If the concern is purely with the fact that you can specify multiple images, then I'd be okay with only accepting a single image, i.e. 'aggregate cache image AGGREGATE IMAGE' | |
| 10:20:42 | stephenfin | People can always invoke it multiple times | |
| 10:21:10 | stephenfin | I really do think positional is the way to go though, and the fact that mriedem said the same (and dansmith didn't argue with him) helps my case | |
| 10:21:54 | gtema | yeah yeah, doing that | |
| 10:25:53 | gtema | nice hint with another patch - haven't seen that | |
| 10:26:28 | openstackgerrit | Stephen Finucane proposed openstack/python-openstackclient master: compute: Fix 'server * -f yaml' output https://review.opendev.org/761205 | |
| 10:26:28 | openstackgerrit | Stephen Finucane proposed openstack/python-openstackclient master: compute: Fix 'usage * -f yaml' output https://review.opendev.org/761595 | |
| 10:26:29 | openstackgerrit | Stephen Finucane proposed openstack/python-openstackclient master: compute: Fix 'server group * -f yaml' output https://review.opendev.org/761596 | |
| 10:26:30 | openstackgerrit | Stephen Finucane proposed openstack/python-openstackclient master: compute: Fix 'hypervisor show -f yaml' output https://review.opendev.org/763004 | |
| 10:26:30 | openstackgerrit | Stephen Finucane proposed openstack/python-openstackclient master: Add support for 'server group create --rule' parameter https://review.opendev.org/761597 | |
| 10:26:30 | openstackgerrit | Stephen Finucane proposed openstack/python-openstackclient master: trivial: Use plural for appended parameters https://review.opendev.org/761598 | |
| 10:26:42 | openstackgerrit | Artem Goncharov proposed openstack/python-openstackclient master: Switch compute aggregate functions to SDK https://review.opendev.org/762134 | |
| 10:26:51 | gtema | here you go | |
| 10:27:00 | stephenfin | thanks :) | |
| 10:27:52 | openstackgerrit | Ananya proposed openstack/ansible-collections-openstack master: migrating routers_info from AnsibleModule to OpenStackModule https://review.opendev.org/763149 | |
| 10:29:33 | gtema | stephenfin, do you think we need to cover reboot/shutdown/start host stuff in SDK? | |
| 10:29:48 | gtema | I think OSC currently doesn't support those | |
| 10:30:26 | stephenfin | The SDK is supposed to be complete, but that API (os-hosts) is deprecated and the things we said to use instead (os-hypervisors, os-services) are present in 2.1 iirc | |
| 10:30:32 | gtema | investing time in deprecated/dropped functionality is hard | |
| 10:30:49 | stephenfin | So I think it's exceptionally low priority, IMO. I wouldn't bother, personally | |
| 10:31:05 | gtema | pretty much same for me | |
| 10:31:22 | gtema | my plan was to get basic host ops covered in SDK to be able to use them from OSC | |
| 10:32:26 | stephenfin | we already have e.g. 'hypervisors list' | |
| 10:32:39 | stephenfin | so I think you're covered there? | |
| 10:32:48 | stephenfin | that'll give you host information | |
| 10:33:04 | gtema | that's true, but if you want to fill 'openstack aggregate add host' with host check you need that | |
| 10:33:19 | gtema | or you mean hypervisors are really giving same info back? | |
| 10:33:42 | stephenfin | the latter; it's the same information exposed by both afaik | |
| 10:33:55 | gtema | hm, interesting | |
| 10:35:12 | stephenfin | To be clear though, that idea was also a nice-to-have UX improvement. Converting more of the other commands to SDK like you've been doing is maybe more helpful? | |
| 10:35:27 | stephenfin | I should start prefixing those comments with 'idea:' or something to distinguish them | |
| 10:36:11 | gtema | perfectly fine with that. I would be perhaps better if I work on SDK switch changes (SDK and OSC) and you improve UX | |
| 10:36:24 | stephenfin | wfm :) | |
| 10:36:48 | gtema | hypervisor vs host are 2 totally different set of APIs (facepalm) | |
| 10:37:01 | gtema | making smooth transition is not easy | |
| 10:37:33 | gtema | especially getting host details is so bad, while getting hypervisor details is ok | |
| 10:39:03 | gtema | you will participate tomorrow in a meeting, right? | |
| 10:39:11 | stephenfin | gtema: We could just deprecate the 'host *' commands | |
| 10:39:19 | gtema | I wanted to agree on the "order" of next switches | |
| 10:39:20 | stephenfin | rather than providing a shim | |
| 10:39:54 | gtema | interesting idea. This would however mean we can't drop novaclient from deps in a long run | |
| 10:40:00 | stephenfin | it's not like we'll break older clouds; those have os-hypervisors and os-services too | |