| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2019-11-06 | |||
| 20:06:09 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/openstacksdk master: "qos_network_policy_id" attribute added to port resource https://review.opendev.org/693247 | |
| 20:26:59 | efried | umbSublime: Shanghai would probably be the right place this week | |
| 20:27:20 | umbSublime | efried: I wish I was there | |
| 20:27:29 | efried | that makes one of us | |
| 20:27:50 | umbSublime | Last summit I was too was the one in Boston | |
| 20:28:30 | efried | that might have been my first one | |
| 20:28:43 | efried | no, that can't be right | |
| 20:29:19 | efried | Austin or Boston | |
| 21:50:09 | openstackgerrit | Michael Johnson proposed openstack/python-openstackclient master: Switch to using osc_lib.utils.tags https://review.opendev.org/662864 | |
| 21:51:00 | johnsom | Would really be good to merge ^^^^ as changes are being made to the old _tag.py that this removes. | |
| 21:55:25 | johnsom | efried So, I see your change to _tag merged that added the "enhance_help parameter to some of the tags methods. I am guessing those need to be added to the new osc_lib version? | |
| 21:56:15 | johnsom | https://www.irccloud.com/pastebin/Gj2aSRX5/ | |
| 21:56:17 | efried | johnsom: ugh, that's an ugly enough hack kept buried in python-openstackclient, I would hate for it to have to bleed over to a different repo | |
| 21:56:49 | efried | I didn't know about that moving to osc-lib | |
| 21:56:49 | johnsom | efried I had removed _tag.py from pythonclient, it has just been sitting for review for a while. | |
| 21:57:06 | johnsom | We moved all of that code to osc_lib/utils/tags.py | |
| 21:57:14 | johnsom | So that other projects could use it | |
| 21:57:29 | efried | Out of curiosity, what other projects would need to use it? | |
| 21:57:36 | efried | Since it's you talking, I'm guessing octavia :P | |
| 21:57:44 | johnsom | Octavia wants it for example | |
| 21:58:07 | johnsom | It would be nice if we all implemented --tags the same... grin | |
| 21:59:12 | johnsom | FYI: https://review.opendev.org/#/c/662859/ | |
| 22:00:26 | johnsom | I'm not sure I quite understand why the lambda is needed, but I just found that the removal patch was in conflict | |
| 22:05:52 | johnsom | This is the patch I just rebased that will likely fail as osc_lib doesn't have the "enhance_help" param: https://review.opendev.org/#/c/662864/ | |
| 22:08:54 | efried | johnsom: I can at least explain how this came about. | |
| 22:09:40 | efried | The networking commands are kind of a mess. The commands and options for neutron and nova-network are interleaved. | |
| 22:09:46 | johnsom | Ok. I am guessing it is part of getting the docs working | |
| 22:10:02 | johnsom | Joy | |
| 22:10:05 | efried | When you're running on the CLI, the client tries to detect which network you're running by looking for the neutron service. | |
| 22:10:13 | efried | and if it doesn't find it, assumes you're running nova-net | |
| 22:10:30 | efried | then it loads up only the commands/options for whichever service it decided you're running. | |
| 22:11:17 | efried | this of course relies on you being authorized to ask the right question to look for the service. But I guess chances are if you can't look at the catalog, you can't do anything else on the CLI, so it may not matter | |
| 22:11:49 | efried | anyway, that didn't affect the docs when they were hardcoded... | |
| 22:11:51 | johnsom | Well, if I remember right, you could run both at the same time | |
| 22:11:59 | efried | oh, well, osc wouldn't let you | |
| 22:12:20 | efried | because if it detected neutron, it would only load up the neutron options and operations. | |
| 22:12:39 | efried | I'm asserting this based on code inspection, not experience btw | |
| 22:13:30 | efried | if you're looking at the HTML docs, on a web server somewhere, we have NO way of knowing whether you care about neutron or nova-net. | |
| 22:13:30 | efried | BUT | |
| 22:13:30 | efried | so my goal was to get the docs generated | |
| 22:13:43 | efried | so we have to show docs for both | |
| 22:13:50 | johnsom | So nova networking is EOL I think, maybe we just have the docs render the neutron version? | |
| 22:14:03 | efried | except osc has to continue to work against old clouds. Forever. | |
| 22:14:23 | efried | ...so I had to come up with a way to make cliff generate both sets of docs | |
| 22:14:40 | efried | and this was before I got to the keystone and glance docs, where I actually ended up completely separating them into different pages. | |
| 22:14:48 | johnsom | It would still *work*... wink, wink | |
| 22:15:17 | efried | ...for e.g. identity v2 vs v3, glance v1 vs v2 | |
| 22:15:48 | efried | but that was a much easier sell in that case because the different versions were handled by completely different code, separated cleanly by package. | |
| 22:16:23 | efried | in order to do that for the networking pieces, one would first have to split them into at least separate classes. | |
| 22:17:43 | efried | In any case, what I ended up doing was implementing this three-way code path: We look for neutron, and if we don't find it because it's not there we assume nova-net, but if we don't find it because we can't even ask the question, we assume we're building docs. | |
| 22:19:11 | efried | If neutron, we only run the methods that add neutron opts. If nova-net, likewise. | |
| 22:19:12 | efried | But for the docs fork, we run both; and everywhere we register an option, we decorate its help with "neutron only" or "nova net only" kind of thing. | |
| 22:20:06 | efried | To come full circle, this means that any code path that registers an option used by networking commands needs a way to understand *when* and *how* to add those decorations. | |
| 22:20:22 | efried | the easiest way was with a callable kwarg | |
| 22:20:44 | efried | so that in fact only the caller has to know when/how. | |
| 22:21:29 | efried | And making the default a no-op lambda was easier than having | |
| 22:21:30 | efried | in each of the methods (tags wasn't the only one) | |
| 22:21:30 | efried | help=enhance_help(_(normal)) | |
| 22:21:30 | efried | else: | |
| 22:21:30 | efried | help=_(normal) | |
| 22:21:30 | efried | if enhance_help is None: | |
| 22:21:43 | johnsom | So, the odd thing is looking at the nova networks API, it doesn't even have tags | |
| 22:21:59 | johnsom | https://docs.openstack.org/api-ref/compute/?expanded=create-allocate-floating-ip-address-detail#create-allocate-floating-ip-address | |
| 22:23:50 | johnsom | Right, ok, so that is this use case. | |
| 22:24:16 | efried | please also keep in mind that I was trying to make $new_docs look as much like $old_docs as possible with this series. I wasn't getting in the business of figuring out where $old_docs were incorrect or whatever. | |
| 22:24:39 | johnsom | Right | |
| 22:25:07 | efried | ...except insofar as $old_docs had gotten left behind when new options were added, kind of thing. | |
| 22:25:17 | johnsom | There is a bunch of bit-rot in both the nova and neutron docs from what I have seen.. Fixing the API-ref for load balancing was a chore | |
| 22:25:23 | efried | which is one of the purposes of this transition: so you don't have to worry about maintaining two places. | |
| 22:26:41 | efried | Is dtroyer the/a maintainer of osc-lib? | |
| 22:27:16 | efried | Probably need to ask him whether he's willing to let this enhance_help kwarg cross the project boundary | |
| 22:27:32 | johnsom | Yeah I think so. He is the one I talked to when I did the tags move to osc_lib | |
| 22:27:41 | efried | Since the default is a no-op, it's probably not the worst thing in the world, shouldn't hurt callers who don't care about it. | |
| 22:28:04 | efried | I would want to see it documented. | |
| 22:28:42 | efried | ...but it looks like you put nice docstrings in the osc-lib incarnations, so yeah, just following that. | |
| 22:29:15 | johnsom | Yeah, I tried to make it better. | |
| 22:29:19 | efried | ++ | |
| 22:29:57 | efried | I guess the way to go about this is to propose that modification to the osc-lib side and use that vehicle to get dtroyer's yea/nay. | |
| 22:30:20 | efried | You want to do that since you did the original, or you want me to do it since I made this mess? (inadvertently, and sorry) | |
| 22:30:32 | johnsom | Well, I didn't do a good job getting his attention to get the python-openstackclient patch merged, so.... | |
| 22:30:40 | dtroyer | tbh I really don't like that approach but I don't have the time to do anything differrent. the entire generated docs is a huge compromise anyway so wtf | |
| 22:30:57 | johnsom | dtroyer Hi Dean | |
| 22:30:59 | dtroyer | johnsom: my apologies for that | |
| 22:32:24 | johnsom | I am wondering for tags if we can make a parameter that is more generic and optional and move forward that way. That way for these edge cases we can pass something over to have an annotation. | |
| 22:32:37 | johnsom | Ok, well, at least I understand the issue now. | |
| 22:32:51 | johnsom | efried Let me noodle on it a bit. | |
| 22:34:15 | efried | dtroyer: Yeah, the right thing is probably to split the neutron/nova-net commands/opts into distinct classes (preferably in distinct packages), but I'm not sure how the CLI would decide which entry points to load up. | |
| 22:34:48 | efried | uhh, maybe the framework is already there, because of how we use --os-*-api-versionZ | |
| 22:36:07 | dtroyer | efried: the problem there is there is nobody to do that work. That also needs to be done for volume v2/v3 and has sat there for 2-3 years now | |
| 22:37:04 | dtroyer | and I am now stuck trying to sort out how to deal with the SDK release breaking the snot out of nodepool and cleaning up the mess that made in my 3ci stacks | |
| 22:39:34 | johnsom | I think we can make the tags code in osc_lib take a version qualifier and produce the same result, just by handling the tags params special on the OSC side. I think a generic parameter like "version_qualifier" would be ok for the shared tags code. | |
| 22:41:16 | efried | johnsom: but then you have to duplicate the text, translation, formatting, and conditioning in osc-lib. | |
| 22:41:30 | efried | I suppose you could just *move* it in there, shrug. | |
| 22:43:16 | johnsom | One question I have is we don't call out api versions on any of the other commands right? We just assume the "not implemented" response is clear enough. | |
| 22:44:12 | efried | what do you mean? | |
| 22:44:52 | johnsom | Well, here: https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/network.html#network-create | |
| 22:45:15 | johnsom | --dns-domain only exists on certain versions of neutron API. We don't call that out here. | |
| 22:45:55 | johnsom | Same with --qos-policy which is only available in certain versions of neutron API and with the neutron extension loaded. | |
| 22:46:24 | efried | Yeah, that would be nice. Not a reason to move backwards on neutron vs nova-net though | |
| 22:47:10 | efried | and btw, the "not implemented" response is not necessarily clear. | |