Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2020-01-13
16:55:52 KeithMnemonic got it thanks, there is a merge conflict so i need to do it by hand anyway
17:24:57 openstackgerrit Dean Troyer proposed openstack/python-openstackclient master: Add unit tests and release note for dns_publish_fixed_ip https://review.opendev.org/702266
18:24:18 openstackgerrit Merged openstack/python-openstackclient master: Raise hacking to more recent 2.0.0 https://review.opendev.org/702019
18:24:20 openstackgerrit Merged openstack/python-openstackclient master: Raise flake8-import-order version to latest https://review.opendev.org/702036
19:03:26 efried johnsom: Before I go digging, can you remind me where we are on the osc-lib business that was holding up https://review.opendev.org/#/c/692931/ ?
19:04:28 efried looks like https://review.opendev.org/#/c/693267/ is still open.
19:04:33 johnsom efried This is stuck: https://review.opendev.org/#/c/662864/
19:04:41 johnsom On this: https://review.opendev.org/#/c/693267
19:05:16 efried johnsom: is it just that we need the latter approved before the former can pick it up properly?
19:05:25 efried approved/released/constrainted/etc?
19:05:52 johnsom I think so, but it has been a long time, so the details are fuzzy
19:10:57 efried johnsom: Can we swap back in? Cause off the top I'm not sure I understand how https://review.opendev.org/#/c/662864/ needing https://review.opendev.org/#/c/693267 should hold up https://review.opendev.org/#/c/692931/
19:14:12 johnsom efried Yeah, the whole issue was the addition of "enhanced_help" you did. It broke other pending patches and is a bit of an oddity. So, the discussion was if we revert the "enhanced_help" change or if we move forward with it and add it to osc-lib.
19:14:32 efried johnsom: but how does that relate to autogenerating docs for identity?
19:14:34 johnsom efried if we reverted, this https://review.opendev.org/#/c/692931/ should not merge either
19:16:01 johnsom efried that patch drops the versioning callout on the --tag options
19:16:28 efried tbc, ...31 doesn't do anything with enhance_help. If we revert that, we would have to revert down that (already merged) series as well.
19:16:40 johnsom Yep
19:16:41 efried if my delta makes the versioning of --tags wrong, that's its own issue.
19:16:48 johnsom It was a pretty nasty mess as I remember
19:19:32 efried johnsom: I'm rechecking to get a new docs build to be sure, but looking at the code, it should be the case that we're only adding the docs for --tags to the v3 doc.
19:19:35 johnsom It looks like Dean was ok with moving forward with the "enhanced_help" path, but it looks like the cores never confirmed that
19:19:37 efried which is what we want to do.
19:19:46 efried johnsom: "the cores" would be... Dean.
19:19:57 johnsom lol, well
19:20:40 efried I think dtroyer was waiting for you to confirm that that solution would work for you.
19:20:47 efried before +2ing
19:20:53 johnsom There are five folks on the list, but yeah, I understand this area is lacking at the moment
19:21:04 efried I actually can't tell who's core in osc-lib
19:21:10 efried is it python-openstackclient-core?
19:21:16 johnsom yes
19:21:20 efried in which case we really only have three: mordred, dtroyer, amotoki
19:21:40 efried Matt is gone, and Doug hasn't been active there for a while.
19:22:35 johnsom So maybe Dean is around and we can just get that osc-lib change rolling
19:23:31 efried johnsom: so let me propose this: If the docs build confirms that --tags only show up in the v3 docs and not in the v2 docs, will you unblock the identity patch? Then we can decide whether to revert the enhance_help business -- in which case we'll need to do *something* else to fix that whole stack -- or approve the osc-lib patch, whereupon https://review.opendev.org/#/c/662864/ can be vetted to make sure it subsumes identity etc.
19:24:42 johnsom I would really like to see a path forward on the centralized tags code before we merge another patch that changes the way we document / handle tags.
19:25:01 efried I get that, but my point is that that ship has already sailed
19:25:09 johnsom That change was pretty big breakage on an already half merged migration
19:25:50 efried For that I apologize; I didn't know that change was in flight, and I was on a (totally unrelated) mission.
19:26:19 efried that said, this kind of conflict is pretty standard fare for this kind of development
19:26:20 johnsom Yeah, I get it.
19:27:52 openstackgerrit Merged openstack/python-openstackclient master: Add dns_publish_fixed_ip attribute to subnets https://review.opendev.org/679834
19:27:53 openstackgerrit Merged openstack/python-openstackclient master: Remove redundant OpenStackShell.prepare_to_run_command https://review.opendev.org/691865
19:31:11 johnsom This is the tie-in with the identity stuff BTW: https://review.opendev.org/#/c/691767/4/openstackclient/identity/common.py
19:32:25 efried got it
19:35:15 efried mordred: you willing to be the second core on this?
19:36:28 efried where "this" is: enhancing osc-lib's handling of --tag options to accept a callable so it can be decorated as pertaining to only a specific version of the API?
19:39:33 efried johnsom: actually, wouldn't it only matter if add_project_domain_option_to_parser had *also* been added to osc-lib? (afaict it wasn't)
19:41:51 efried And that guy is using enhance_help to decorate the help as pertaining only to neutron (as opposed to a particular version)
19:42:56 johnsom Wasn't this all because you didn't want to fix the code structure to deal with the mixed neutron/nova commands?
19:43:13 efried that's where it started, yes.
19:43:43 johnsom The new docs system can't handle the current method and some projects were using static pages.
19:43:45 johnsom Right.
19:44:38 efried where "fix the code structure" would have entailed splitting neutron and nova-network classes into 100% separate packages, like what we see for identity
19:46:16 efried doing all of this -- enhance_help in python-openstackclient and osc-lib and reworking your change -- is still an order of magnitude less effort than that would be.
19:46:35 efried Arguably "the right thing", but... 80/20 rule.
19:47:05 johnsom Yeah, and I agreed that I can live with that, we just need to commit to it or revert IMO
19:49:17 efried that's fine, but I still don't agree that that should hold up https://review.opendev.org/#/c/692931/ (and its successor) which have nothing to do with enhance_help
19:50:14 efried But that's your prerogative. So let's just push on dtroyer mordred amotoki to merge https://review.opendev.org/#/c/693267/ so we can move forward.
19:50:23 johnsom Well, once the docs re-render maybe we can look at the changes and issues that related them. I think that if we revert, we will want to stick with the static pages until a new path forward is found
19:52:43 efried The identity patch wouldn't be affected by that, though.
19:52:44 efried the fact that there's an enhance_help in an identity lib is coincidental; it's only used by networking.
19:52:44 efried that method is called in a hundred other places, but the enhance_help kwarg is only used from networking
19:52:44 efried so whichever approach we choose, the identity docs split/autogen patch wouldn't be different.
20:04:36 efried dtroyer: tl;dr:
20:04:37 efried You +2ed the glance doc autogen patch https://review.opendev.org/#/c/693025/ and wondered why it hadn't been approved earlier
20:04:37 efried I mentioned its predecessor https://review.opendev.org/#/c/692931/ was held up
20:04:37 efried So I was discussing with johnsom whether that holdup was valid
20:04:37 efried But whether it is or not, we would like to move the osc-lib enhance_help patch https://review.opendev.org/#/c/693267/ along
20:09:23 dtroyer efried: right, I figured that was the case (after I looked at them in the wrong order). I'm getting to going ahead with merging 693267, but having to do something like this is a clue that maybe we should just copy that code instead now. DRY is good, until it isn't, and OSC has taught me that sometimes the duplication is work it in the end… thoughts to inform the future, I think we're going to finish this path at least for now
20:11:17 efried dtroyer: ack, and thanks.
20:11:17 efried to be clear, which piece would duplication have been better for in this scenario?
20:14:51 dtroyer with the changes to help, I would have re-thought moving the tags stuff to osc-lib in the first place. In this case though that is being done to make it available to plugins… there is a school of thought that it is the plugins that should be eliminated so that would be an alternative someday
20:16:52 dtroyer I just +W https://review.opendev.org/#/c/693267/, we'll need to do an osc-lib release so OSC can pick that up.
20:17:08 efried gotcha. Thanks. johnsom ^
20:35:38 johnsom Sorry, had a meeting, catching up
20:37:55 johnsom dtroyer Yeah, I think in the case of tags, it's a good thing that, plugins or not, we are using common code for tags. This is an area that is easy to start diverging in the commands.... This was part of why I proposed moving it into osc-lib and volunteered to do the work.
21:10:53 openstackgerrit Dean Troyer proposed openstack/python-openstackclient master: Add unit tests and release note for dns_publish_fixed_ip https://review.opendev.org/702266
21:26:50 KeithMnemonic smcginnis: it passed! first hurdle done
21:26:57 smcginnis Finally!
21:55:05 openstackgerrit Merged openstack/osc-lib master: Add enhance_help kwarg to tags option generators https://review.opendev.org/693267
22:17:37 efried smcginnis: Did we decide the first release after dropping py2 needs to be a 'major'?
22:21:49 efried johnsom, dtroyer: https://review.opendev.org/702302 <== osc-lib 2.0.0 release (smcginnis assumed ^ yes)
22:22:15 johnsom Yes, dropping py2 is a major bump
22:23:10 smcginnis efried: Sorry - correct, that's considered a backwards incompatible change and needs the major version bump.
22:23:27 efried cool, thanks.
22:24:23 johnsom There should have been a release note for the py2 drop as well, but it doesn't look like that happened
22:28:55 efried johnsom: Doesn't need to be a separate release. I incorporated it in the same patch. smcginnis: is there any reason the first release after dropping py2 needs to be immediately after that patch?
22:29:45 efried johnsom: in other news, the new build for https://review.opendev.org/#/c/692931/ is available. I linked to the 'project' subcommand pages, please confirm whether the presence/absence of --tag is appropriate there.
22:30:15 smcginnis efried: You mean if it should be released right away after making the changes in the cycle goal versus waiting for a bit and picking up some other changes?
22:30:25 efried smcginnis: yes, that's what I mean.
22:30:34 smcginnis No, no real reason to have to do it right away.
22:30:56 smcginnis Of course the sooner it's out there, the sooner others will pick it up and find out if there are any issues, but no reason that has to happen immediately.
22:30:57 efried Or rather, in this case, given that other changes are already available, should I force an artificial major release in between so I can immediately push a 2.1.0 for the subsequent feature?
22:31:09 efried ack, so I think we're good here.
22:31:15 johnsom Yeah, release often is good actually.
22:31:19 openstackgerrit Merged openstack/python-openstackclient master: Create Volume v3 functional tests https://review.opendev.org/695044
22:31:24 smcginnis Yeah, it's fine if there are other feature additions along with the py2 drop.
22:31:39 efried cool
22:31:55 smcginnis The major bump is just a signal to downstream consumers of "hey, might want to check what changed here before you upgrade to this one" ;)

Earlier   Later