| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2020-03-26 | |||
| 15:19:18 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Add requires-python entry indicating 3.6 is required https://review.opendev.org/715243 | |
| 15:19:32 | mordred | vphilippon: yeah - I think we can remove 0.44 once 0.44.1 is there | |
| 15:20:00 | vphilippon | Oh, that works too! | |
| 15:20:41 | vphilippon | Even better, taking note of that option (this not the first time I get to report this to maintainers, sadly) | |
| 15:20:51 | mordred | smcginnis, gtema: ^^ see discussion and patch - let's land that and cut an 0.44.1 real quick | |
| 15:21:00 | mordred | actually - let me add a release note explanation real quick | |
| 15:21:12 | gtema | sure, but we can't unless nodepool build are fixed ;) | |
| 15:21:37 | gtema | perhaps also removing "Programming Language :: Python :: 2" | |
| 15:22:57 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Add requires-python entry indicating 3.6 is required https://review.opendev.org/715243 | |
| 15:23:03 | vphilippon | Thanks for the quick response btw :) | |
| 15:23:03 | smcginnis | mordred: Good on the new release. Not so sure about removing anything. That's generally bad. And not clear what the issue is here. | |
| 15:23:03 | mordred | ++ | |
| 15:23:32 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Add requires-python entry indicating 3.6 is required https://review.opendev.org/715243 | |
| 15:23:58 | gtema | yupp | |
| 15:24:01 | mordred | smcginnis: so - the situation is that we neglected adding requires-python>=3.6 to the first release where that was actually physically true | |
| 15:24:18 | mordred | which means python2 installs of openstack with no versions are finding it and trying to install it | |
| 15:24:28 | smcginnis | I'd say only about 50% (or less) of our ussuri deliverables have done that. | |
| 15:24:51 | mordred | yeah- but openstacksdk is a much wider audience of people who pip install it | |
| 15:24:57 | mordred | since it's end-user and not operator | |
| 15:25:00 | smcginnis | I guess openstacksdk is probably more prevalent outside of openstack, so we can't rely on upper-constraints. | |
| 15:25:02 | mordred | yeah | |
| 15:25:03 | smcginnis | Yeah | |
| 15:25:05 | smcginnis | ;) | |
| 15:26:05 | mordred | so - in this case, I think releasing a quick 0.44.1 bugfix and then removing 0.44.0 from pypi will unbreak people - as long as we wait for u-c to bump to 0.44.1 in openstack before doing so | |
| 15:26:14 | mordred | obviously a discussion for release team | |
| 15:26:30 | mordred | and first step is getting the fix landed so we can release 0.44.1 which is uncontroversial | |
| 15:26:53 | mordred | gtema: I think maybe let's disable the nodepool test for the requires-python fix so we can get it landed, then re-enable | |
| 15:27:07 | gtema | ok, agree | |
| 15:27:25 | gtema | but then also have otc vendor profile fix in | |
| 15:28:25 | mordred | ++ | |
| 15:28:32 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Add requires-python entry indicating 3.6 is required https://review.opendev.org/715243 | |
| 15:28:32 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Re-add nodepool functional test https://review.opendev.org/715245 | |
| 15:28:33 | gtema | cool | |
| 15:28:44 | mordred | gtema: you wanna rebase otc patch on requires-python patch? | |
| 15:28:50 | gtema | sure | |
| 15:29:44 | vphilippon | An alternative to having to remove 0.44.0 would be to re-release 0.43.0 as 0.44.1, and release the "original" 0.44.0 with the metadata, as 0.45.0. | |
| 15:30:07 | gtema | oh, this would be too complicated | |
| 15:30:16 | mordred | smcginnis: I -2'd the UC bump for sdk: https://review.opendev.org/#/c/715199/ | |
| 15:30:20 | mordred | actually ... | |
| 15:30:27 | vphilippon | I believe so, but I share the option :) | |
| 15:30:38 | mordred | I think I like that idea, weirdly | |
| 15:31:08 | gtema | 43 with content of 44? | |
| 15:31:13 | mordred | actually | |
| 15:31:45 | mordred | I think it should be "revert furturist patch. land. release. unrevert and add requires-python. land. release" | |
| 15:32:16 | gtema | eeh, project cleanup now depends on futurist | |
| 15:32:34 | mordred | yeah. lemme see if I can make a patch real quick - we can look at it in parallel and see which we like | |
| 15:34:45 | vphilippon | I'd say if you are able to act quickly, the option of removing 0.44.0 remains sane-ish. If times has to pass (for whatever reason, build system, discussion, etc.), Then the revert-fix-release pattern by republishing older version as newer becomes better, IMHO | |
| 15:35:33 | vphilippon | I'm not a maintainers here though, simply sharing my pip experience expertise here. | |
| 15:40:12 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Revert "Switch to futurist for concurrency" https://review.opendev.org/715249 | |
| 15:40:13 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Revert "Revert "Switch to futurist for concurrency"" https://review.opendev.org/715250 | |
| 15:40:36 | mordred | gtema: ^^ I think we can get both of those landed and released today | |
| 15:40:53 | dtantsur | and the cleanup patch too? | |
| 15:41:05 | dtantsur | I wonder if it's much easier to re-release the same hash as a newer version | |
| 15:41:16 | mordred | then the release notes would be very strange :) | |
| 15:41:26 | dtantsur | maybe :) | |
| 15:41:34 | dtantsur | you can try it locally with `git tag` and see | |
| 15:41:49 | mordred | dtantsur: I modified the revert so that we don't need to revert the cleanup patch - it just won't use futurist for it for that one release | |
| 15:42:03 | mordred | dtantsur: k. I'll try that real quick | |
| 15:42:22 | gtema | and actually nobody will use it so far (likely) | |
| 15:42:28 | mordred | dtantsur: considering we just released the cleanup feature today - I think if we rleease 0.45 and 0.46 back to back - it should be safe | |
| 15:42:53 | dtantsur | mordred: maybe we can/should default to concurrent.futures.ThreadPoolExecutor actually? | |
| 15:43:08 | dtantsur | the same API as futurist, so people can switch easily? | |
| 15:44:50 | mordred | dtantsur: oh - that's not a terrible idea | |
| 15:45:09 | gtema | keep your thought, otherwise mordred will create 30 different patches and we will die ensuring we approve them in correct order ;) | |
| 15:45:12 | mordred | people who want to use futurist.GreenThreadExecutor can still pass it | |
| 15:45:18 | dtantsur | exactly | |
| 15:45:38 | mordred | I think I like that even more. it's basically what we have above except without the second re-revert | |
| 15:45:48 | mordred | maybe should rewrite the release note slightly differently | |
| 15:46:09 | tremble | mordred, Should doing dict(some_resource) result in a *all* parameters (body and URI based) showing up in the new dict ? | |
| 15:46:41 | mordred | tremble: that's an excellent question - I believe so? | |
| 15:47:37 | mordred | dtantsur: the re-tag idea did not work :) | |
| 15:47:42 | gtema | oh, I remember some interesting stuff about that when fighting with replacing cloud layer - something funny was there for sure | |
| 15:47:58 | dtantsur | mordred: oookay, let's go with this patch | |
| 15:48:10 | dtantsur | (the release note could use an update indeed) | |
| 15:49:31 | gtema | tremble, we prefer using https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/resource.py#L918 (to_dict) for those reasons | |
| 15:50:14 | tremble | gtema, That'll do me as a fix :) | |
| 15:50:53 | tremble | gtema, Would misbehaving with dict() be considered a bug? | |
| 15:51:05 | tremble | and if so, where do you want reports :) | |
| 15:51:16 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Revert "Switch to futurist for concurrency" https://review.opendev.org/715249 | |
| 15:51:50 | mordred | dtantsur, gtema: ^^ I think that should fix the issue. we can land that, gtema's otc fix and then cut an 0.45 | |
| 15:51:53 | gtema | tremble - I would not say it is a bug, resource implementation is incredible woodoo | |
| 15:52:04 | mordred | then I still think we should put ina. requires-python for 0.46 | |
| 15:52:09 | tremble | gtema, I noticed :) | |
| 15:52:39 | mordred | smcginnis: per scrollback - I think we found a solution that doesn't require removing anything | |
| 15:53:07 | mordred | vphilippon: thanks for the report! we shoudl have a fixed release up soonish (will take a couple of hours for all of the gears to crank) | |
| 15:53:27 | gtema | mordred, so I rebase vendor patch on top of 715249, right? | |
| 15:54:45 | smcginnis | I will watch for the release and try to get it through asap. | |
| 15:54:51 | smcginnis | And the requirements update. | |
| 15:55:11 | vphilippon | @mordred: Alrighty, thanks a lot for the quick reaction. And thanks to everyone else too :) | |
| 15:55:54 | tremble | gtema, to_dict() still seems to exclude the resource.URI entry | |
| 15:56:29 | gtema | ah, and you need those explicitely? | |
| 15:56:49 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Change default image type in the OTC vendor profile https://review.opendev.org/715183 | |
| 15:56:49 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Add requires-python entry indicating 3.6 is required https://review.opendev.org/715243 | |
| 15:57:03 | mordred | gtema: rebase done ^^ | |
| 15:57:09 | gtema | ok, thks mordred | |
| 15:57:17 | mordred | gtema: I'm guessing this is for returning data to an ansible module - which needs json-able dict | |
| 15:57:40 | mordred | smcginnis: I think the constraints patch that's there for us already is now safe | |
| 15:58:15 | mordred | smcginnis: but - we can also just wait to reduce churn | |
| 15:58:21 | mordred | smcginnis: I'll get release patches up asap | |
| 15:58:34 | smcginnis | OK. I need to update that so it's python_version aware. Might as well wait a bit. | |