Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2019-11-06
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 johnsom efried I had removed _tag.py from pythonclient, it has just been sitting for review for a while.
21:56:49 efried I didn't know about that moving to osc-lib
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 so my goal was to get the docs generated
22:13:30 efried BUT
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: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 if enhance_help is None:
22:21:30 efried help=_(normal)
22:21:30 efried else:
22:21:30 efried help=enhance_help(_(normal))
22:21:30 efried in each of the methods (tags wasn't the only one)
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.
22:47:49 efried as an osc noob I had a hell of a time figuring out that my under-the-covers microversion wasn't adequate, and how to remedy it.
22:48:27 johnsom I'm just noticing that this is the first time we have added any version information to the python-openstackclient docs.
22:48:28 efried Can't imagine someone trying to figure out why they can't use the --subnet flag
22:48:35 efried oh, no it's not at all
22:48:53 efried The previous version of these same docs had these same decorations.
22:48:54 efried stand by
22:49:00 johnsom Weil, in server create, the micro-versions aren't called out for those commands
22:49:06 efried https://docs.openstack.org/python-openstackclient/train/cli/command-objects/network.html#network-create

Earlier   Later