opendevreview | Merged openstack/python-openstackclient master: Add four new network agent types to the list command filter https://review.opendev.org/c/openstack/python-openstackclient/+/942175 | 01:43 |
---|---|---|
opendevreview | Merged openstack/openstacksdk master: exceptions: Deprecate http_status, request_id params https://review.opendev.org/c/openstack/openstacksdk/+/929017 | 01:43 |
opendevreview | Merged openstack/openstacksdk master: tests: Rename cloud name variables https://review.opendev.org/c/openstack/openstacksdk/+/942712 | 02:14 |
opendevreview | Merged openstack/openstacksdk master: identity: Fix limit, registered limit creation https://review.opendev.org/c/openstack/openstacksdk/+/942818 | 02:14 |
opendevreview | Merged openstack/codegenerator master: BS volume bootable parameter is a string https://review.opendev.org/c/openstack/codegenerator/+/942920 | 08:08 |
opendevreview | Merged openstack/codegenerator master: Add required to compute service responses https://review.opendev.org/c/openstack/codegenerator/+/942934 | 08:09 |
opendevreview | Rajesh Tailor proposed openstack/python-openstackclient master: Add support for image properties in server show https://review.opendev.org/c/openstack/python-openstackclient/+/940799 | 08:40 |
opendevreview | Rajesh Tailor proposed openstack/openstacksdk master: Add support for image properties in server show https://review.opendev.org/c/openstack/openstacksdk/+/940798 | 08:41 |
priteau | Hello stephenfin. The kayobe CLI broke yesterday following the release of cliff 4.9.0. I have opened a bug with more details here: https://bugs.launchpad.net/kayobe/+bug/2100579. I don't know yet if this is a regression in cliff or if kayobe is using it wrongly. | 08:46 |
stephenfin | priteau: Thanks. I'll take a look now | 10:50 |
stephenfin | priteau: So two things. First, it's a definite logic mistake in cliff: the filter and list comprehensions are not the same thing. I'll fix that now. | 11:13 |
stephenfin | Secondly: the CommandHook class stated that get_parser method should return an ArgumentParser instance, while get_epilog should return a string. Kayobe's 'HookDispatcher' is returning None for both. | 11:14 |
stephenfin | But, seeing as that's been tolerated before and there was the filter for the epilog, I'm inclined to think that cliff's own docstrings were wrong | 11:15 |
opendevreview | Stephen Finucane proposed openstack/cliff master: command: Filter on empty epilog, not empty hooks https://review.opendev.org/c/openstack/cliff/+/942992 | 11:38 |
opendevreview | Stephen Finucane proposed openstack/cliff master: hooks: Update type hints to reflect reality https://review.opendev.org/c/openstack/cliff/+/942993 | 11:38 |
stephenfin | gtema, priteau: ^ | 11:38 |
priteau | stephenfin: I have just tested 942992, it resolves the issue in kayobe | 11:50 |
priteau | Would you be able to push a new release soon? | 11:51 |
stephenfin | Yes, I'd just like gtema to sanity check it, then we can merge and cut a 4.9.1 release | 11:51 |
gtema | sure | 11:52 |
stephenfin | https://review.opendev.org/c/openstack/releases/+/942996 There's a release patch. We'll just recheck it once those patches are merged. | 11:53 |
priteau | stephenfin: Would you still recommend we fix the HookDispatcher class? I can look into it once our CI is green again. | 12:02 |
stephenfin | priteau: You can fix your CI now by returning '' from get_epilog https://paste.opendev.org/show/bgLM8MsGNB0sWVVtjzsS/ | 12:04 |
stephenfin | As an alternative to capping and uncapping | 12:04 |
priteau | Any CI fix we do ourselves involves fixing the oldest branches first, because we have upgrade jobs that are failing. We don't use upper constraints in kayobe so a new release would magically fix all our branches :) | 12:06 |
priteau | Then I can fix get_epilog in master and backport the usual way | 12:07 |
stephenfin | priteau: Okay, then maybe hold tight until this evening and we'll hopefully be able to get that fix out sharpish | 12:18 |
stephenfin | otherwise, IMO you might as well apply the fix to all branches rather than a cap followed by an uncap. It's literally half the work :) | 12:18 |
gtema | stephefin - I assume we may want to add explicit test like I posted in releases channel for capturing regressions | 12:19 |
stephenfin | sorry, what explicit test? | 12:19 |
gtema | https://paste.openstack.org/show/byZpxP2y2ujhv4TCKehT/ | 12:19 |
gtema | something like that | 12:19 |
gtema | just convert print to assert | 12:19 |
stephenfin | ah, yes | 12:19 |
gtema | I assume you have idea how to fix the regression assuming you introduced this change? You should have had reasons for that | 12:20 |
stephenfin | Yeah, I'll come up with something. I first trying to figure out why it has that impact though | 12:21 |
gtema | great, I am afk for now, but feel free to ping me | 12:22 |
stephenfin | As for why I changed that, we're asserting that if you give e.g. data_type=str, the output will a str | 12:23 |
stephenfin | If we just returned the value the user provided without doing anything to it, you do e.g.: | 12:23 |
stephenfin | _conver_type(123, data_type=list) | 12:24 |
stephenfin | but the result will be the int 123 | 12:24 |
stephenfin | which is obviously wrong, and mypy complained as such | 12:24 |
stephenfin | hence the change. and the horrific type hints :) | 12:25 |
stephenfin | gtema: Okay, there's a bigger issue with _convert_type here. It popped up in older releases if you passed list_type also. For example https://paste.opendev.org/show/bZOYPdh75lIJLal2P65T/ | 12:36 |
stephenfin | idk if we should/can support this, at least while continuing to offer type conversion :-\ | 12:37 |
stephenfin | tbc, the issue is that we're using this inside '__getattribute__', and '_convert_type' is occasionally returning a _copy_ of a field, rather than the field itself | 12:41 |
stephenfin | Thus, attempts to modify a field in-place modify the copy rather than the "source" | 12:42 |
stephenfin | To fix this properly, I think we need | 12:42 |
stephenfin | ...to kill _convert_type | 12:42 |
gtema | We should rather redesign everything from OpenApi anyway | 12:44 |
stephenfin | https://review.opendev.org/c/openstack/horizon/+/943001 | 13:22 |
opendevreview | Stephen Finucane proposed openstack/openstacksdk master: Support server unshelve to specific availability zone https://review.opendev.org/c/openstack/openstacksdk/+/942877 | 13:52 |
opendevreview | Stephen Finucane proposed openstack/openstacksdk master: fields: Save converted attributes before returning https://review.opendev.org/c/openstack/openstacksdk/+/943009 | 15:02 |
stephenfin | gtema: ^ | 15:02 |
opendevreview | Merged openstack/cliff master: command: Filter on empty epilog, not empty hooks https://review.opendev.org/c/openstack/cliff/+/942992 | 15:19 |
opendevreview | Merged openstack/cliff master: hooks: Update type hints to reflect reality https://review.opendev.org/c/openstack/cliff/+/942993 | 15:19 |
opendevreview | Douglas Viroel proposed openstack/openstacksdk master: Bump compute max microversion to 2.100 https://review.opendev.org/c/openstack/openstacksdk/+/938833 | 16:26 |
priteau | stephenfin: Thanks a lot for the quick fix. It looks like our CI is green again. | 16:54 |
opendevreview | Artem Goncharov proposed openstack/codegenerator master: Fix network external_gateway_info schema https://review.opendev.org/c/openstack/codegenerator/+/943036 | 17:34 |
opendevreview | Artem Goncharov proposed openstack/codegenerator master: Bootstrap magnum OpenAPI build https://review.opendev.org/c/openstack/codegenerator/+/943058 | 20:03 |
opendevreview | Artem Goncharov proposed openstack/codegenerator master: Bootstrap magnum OpenAPI build https://review.opendev.org/c/openstack/codegenerator/+/943058 | 20:11 |
opendevreview | Artem Goncharov proposed openstack/codegenerator master: Bootstrap magnum OpenAPI build https://review.opendev.org/c/openstack/codegenerator/+/943058 | 20:13 |
opendevreview | Manuel Osorio proposed openstack/openstacksdk master: Add share transfer to shared file system https://review.opendev.org/c/openstack/openstacksdk/+/925653 | 20:25 |
opendevreview | Artem Goncharov proposed openstack/codegenerator master: Bootstrap magnum OpenAPI build https://review.opendev.org/c/openstack/codegenerator/+/943058 | 20:43 |
opendevreview | Manuel Osorio proposed openstack/openstacksdk master: Add share transfer to shared file system https://review.opendev.org/c/openstack/openstacksdk/+/925653 | 21:52 |
opendevreview | Oria Weng proposed openstack/python-openstackclient master: [DNM] Identity: Migrate 'limit' commands to SDK https://review.opendev.org/c/openstack/python-openstackclient/+/936279 | 21:56 |
opendevreview | Oria Weng proposed openstack/python-openstackclient master: [DNM] Identity: Migrate 'limit' commands to SDK https://review.opendev.org/c/openstack/python-openstackclient/+/936279 | 22:03 |
opendevreview | Oria Weng proposed openstack/python-openstackclient master: [DNM] Identity: Migrate 'limit' commands to SDK https://review.opendev.org/c/openstack/python-openstackclient/+/936279 | 22:03 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!