| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2019-11-06 | |||
| 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 | |
| 22:49:54 | efried | no, microversions aren't. That would arguably be a nice enhancement. Ideally it would somehow tie back into the client code to discover that automatically, rather than being hardcoded. | |
| 22:50:24 | efried | I'm just saying for neutron vs nova-net, it *was* called out, which is why I didn't see it being a viable path forward to stop doing that when the docs were generated. | |
| 22:51:12 | efried | also fwiw https://docs.openstack.org/python-openstackclient/train/cli/command-objects/role.html identity docs had v2 vs v3 markers. | |
| 22:51:21 | johnsom | were those static pages before? | |
| 22:51:22 | efried | that's major version of course, not microversion. | |
| 22:51:24 | efried | yes | |
| 22:51:29 | johnsom | ah | |
| 22:52:41 | efried | oh, and the way I found out this was even a thing was by trying to build the docs by running both paths. That gives errors on conflicting opts. If you look at the rest of the offending patch, you'll see where I had to put in some pretty weird "if docs build" conditions in some places. | |
| 22:53:04 | johnsom | Yeah, I saw those | |
| 22:53:47 | efried | anyway, as dtroyer implied, there's a right way and there's an expedient way. Unless you're signing up to do the former (I'm not) we're pretty much stuck doing the latter, even though it's kind of distasteful. | |
| 22:54:30 | johnsom | I think we can leave that nastiness in OSC. I'm just working on a way to allow passing that stuff into osc_lib in a more clean way, focused on the tags library for now. | |
| 22:54:58 | johnsom | Well, no good dead goes unpunished. I volunteered to do the tags library work... lol | |