| ralonsoh | stephenfin, hello! if you have some minutes: https://review.opendev.org/c/openstack/openstacksdk/+/927779 | 05:41 |
|---|---|---|
| ralonsoh | thanks in advance! | 05:41 |
| opendevreview | Rodolfo Alonso proposed openstack/openstacksdk master: Add ``trunk_details`` to ``ports`` resource https://review.opendev.org/c/openstack/openstacksdk/+/926609 | 09:02 |
| opendevreview | Rodolfo Alonso proposed openstack/python-openstackclient master: Add the trunk subports information to the port list command https://review.opendev.org/c/openstack/python-openstackclient/+/926611 | 09:58 |
| opendevreview | Rodolfo Alonso proposed openstack/python-openstackclient master: Add the trunk subports information to the port list command https://review.opendev.org/c/openstack/python-openstackclient/+/926611 | 10:07 |
| stephenfin | ralonsoh: I'm slightly concerned that we now have two similarly named methods that will only work with some services | 11:27 |
| stephenfin | How come neutron is using POST instead of PUT? Is PUT an option? | 11:28 |
| stephenfin | And what happens if you POST multiple times? Does that result in it being overridden or...? | 11:28 |
| ralonsoh | stephenfin, PUT was the initial implementation. Was used to add a single tag to a resource. But there are some problems with the tag names, for example if you use slash / | 11:38 |
| ralonsoh | post multiple times: that is tested in the patch. This is idempotent | 11:38 |
| ralonsoh | you can post several times the same tag but it will be created once only | 11:38 |
| ralonsoh | POST is the new method and in the CLI it will replace PUT | 11:39 |
| stephenfin | I thought the issue was that we were passing the parameters in the URI rather than in the body? | 11:40 |
| ralonsoh | yes | 11:40 |
| stephenfin | (btw, I realize I'm straying into Neutron API design rather than openstacksdk stuff here, but I'd like to understand this so please bear with me :)) | 11:40 |
| stephenfin | Okay, how come we didn't just...start passing it in the body? | 11:40 |
| stephenfin | iiuc, that's what nova et al do? | 11:40 |
| ralonsoh | that is the main reason: not to add the tag name in the URL | 11:40 |
| stephenfin | https://docs.openstack.org/api-ref/compute/#replace-tags | 11:41 |
| ralonsoh | that is different: with this call you will replace the existing tag list with the new one passed | 11:42 |
| ralonsoh | with post you'll only create the non existing ones | 11:42 |
| ralonsoh | in the Neutron CLI we are currently reading the current tags and creating the new tag list | 11:42 |
| ralonsoh | instead with this it won't be needed first to read the tag list, you can directly POST the new tags | 11:43 |
| stephenfin | Okay, gotcha. And I'm guessing it's still not possible to PUT with a body? | 11:43 |
| ralonsoh | (and yes, another option could have been updating the neutron PUT command) | 11:43 |
| ralonsoh | yes, this is another option too | 11:43 |
| ralonsoh | I didn't go this way | 11:44 |
| ralonsoh | but of course I can implement that too | 11:44 |
| stephenfin | I'd be interested in knowing how many, if any, of the current methods in the TagMixin actually work with neutron resources | 11:45 |
| stephenfin | i.e. https://review.opendev.org/c/openstack/openstacksdk/+/927779/7/openstack/common/tag.py | 11:45 |
| stephenfin | If the answer is zero, or nearly zero, I wonder if neutron should have it's own TagMixin? | 11:45 |
| ralonsoh | stephenfin, sorry, I don't understand the question | 11:45 |
| ralonsoh | all Neutron resources that have tags (except for the bug with qos policies) work with these methods | 11:46 |
| stephenfin | Can I do e.g. x = network.find('foo'); x.add_tag(session, 'bar') | 11:46 |
| stephenfin | or x.remove_tag(session, 'bar') | 11:46 |
| stephenfin | or x.check_tag(session, 'bar') | 11:46 |
| ralonsoh | that must work, for sure, this patch has the corresponding functional tests | 11:47 |
| ralonsoh | and I think we are testing all these methods | 11:47 |
| ralonsoh | well, we are not testing all of them | 11:48 |
| ralonsoh | let me add this in my TODO list: add FTs for Neutron resources in SDK | 11:48 |
| ralonsoh | add FTs for tags for Neutron resources in SDK* | 11:49 |
| ralonsoh | stephenfin, is that ok for you? | 11:49 |
| stephenfin | sure | 11:50 |
| * stephenfin quickly experiments locally | 11:51 | |
| greatgatsby | Hello. Is it possible to have the openstack commandline client perform pagination automatically, or force it to return all the results in some way? I believe --limit -1 does not work anymore? Thanks for your help. | 11:51 |
| stephenfin | greatgatsby: Pagination should happen automatically. Could you test with OSC < 6.1.0 (when we switched from novaclient to SDK under the hood). I don't have an environment with enough VMs running to test. | 11:53 |
| greatgatsby | stephenfin: thanks, I'll test that version. With 6.3.0, I get a 400 Client Error Invalid input for query parameters limit, Value -1 does not match '^[0-9]*$'. So it looks like it's the API that does not support this anymore, but OSC still shows -1 as a valid argument in the help. | 11:56 |
| greatgatsby | this could also apply to listing volumes, for example, so I'm not sure it's specific to novaclient | 11:58 |
| stephenfin | ralonsoh: Mea culpa. I misunderstood the bug /o\ I assumed this was a replacement for 'set_tag' method, but in reality you are using this as a replacement for the 'add_tag' method (and an alternative to 'set_tag' if you want to keep the extend rather than replace the existing tags) | 11:59 |
| stephenfin | Right? | 12:00 |
| stephenfin | greatgatsby: I'd have to go back and check, but my guess is the server never supported this and instead the client was doing something special if -1 was provided. | 12:00 |
| stephenfin | actually, that was a quick grep. Yeah, this is a client-side thing https://github.com/openstack/python-novaclient/blob/3add6476fb265626d77704f7828dacc21cff81d9/novaclient/v2/servers.py#L1042-L1043 | 12:01 |
| greatgatsby | oh interesting. So this is possibly something that could get "fixed" in the current OSC? | 12:02 |
| stephenfin | greatgatsby: what happens if you omit --limit entirely? From reading that it appears if no limit parameter is provided then everything would have been returned. | 12:03 |
| stephenfin | i.e. --limit -1 is identical to not providing --limit | 12:03 |
| greatgatsby | with 6.3.0 I'm 99% sure we have to use pagination. If we don't provide limit or marker, we get a subset of all the servers | 12:04 |
| ralonsoh | stephenfin, yes, we want to replace set_tag with add_tag, that is idempotent | 12:04 |
| stephenfin | ralonsoh: Okay, Apologies for the confusion. In that case, new suggestion: s/create_tags/add_tags/ (plural) Tags aren't really a resource in their own right so you're not really "creating" anything, anymore than you'd be "creating" a name or name alias. | 12:07 |
| stephenfin | I'd also still be in favour of providing a Neutron-specific TagMixin subclass with this in it, since afaik no other service has this API currently | 12:07 |
| stephenfin | greatgatsby: Right you are. There's also this code https://github.com/openstack/python-novaclient/blob/3add6476fb265626d77704f7828dacc21cff81d9/novaclient/v2/servers.py#L1068-L1069 | 12:08 |
| stephenfin | So the behavior now is that pagination will happen by default because that's what SDK default to. You'd have to opt-in to a limit if you wanted one | 12:08 |
| ralonsoh | stephenfin, I'll update the patch today | 12:09 |
| greatgatsby | stephenfin: ok, thanks a lot for the quick feedback. I'll look deeper into the SDK and investigate my options. Cheers. | 12:10 |
| stephenfin | We should definitely re-add support for -1, at least temporarily. Whether we want to revert to pagination by default is a more difficult thing to decide. IMO, you'd always want to get everything unless you're in a huge environment, in which case you'll quickly learn to pass --limit | 12:11 |
| stephenfin | greatgatsby: nw. If you can file a bug, please do. Links to the bug trackers are in the channel description | 12:11 |
| greatgatsby | stephenfin: will do, thanks again | 12:17 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate keystoneauth1.discover https://review.opendev.org/c/openstack/keystoneauth/+/928814 | 15:46 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate keystoneauth1.plugin https://review.opendev.org/c/openstack/keystoneauth/+/930762 | 15:46 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate keystoneauth1.session https://review.opendev.org/c/openstack/keystoneauth/+/930763 | 15:53 |
| opendevreview | Merged openstack/keystoneauth master: trivial: Consistent f prefixes https://review.opendev.org/c/openstack/keystoneauth/+/930764 | 15:58 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate keystoneauth1.adapter https://review.opendev.org/c/openstack/keystoneauth/+/930765 | 15:58 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate keystoneauth1._fair_semaphore https://review.opendev.org/c/openstack/keystoneauth/+/930766 | 16:08 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate various plugin modules https://review.opendev.org/c/openstack/keystoneauth/+/930767 | 16:10 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate keystoneauth1._utils https://review.opendev.org/c/openstack/keystoneauth/+/930768 | 16:10 |
| opendevreview | Merged openstack/keystoneauth master: typing: Annotate keystoneauth1.exceptions https://review.opendev.org/c/openstack/keystoneauth/+/930769 | 16:10 |
| opendevreview | Oria Weng proposed openstack/python-openstackclient master: Identity: Migrate 'role' commands to SDK https://review.opendev.org/c/openstack/python-openstackclient/+/929712 | 21:14 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!