| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-15 | |||
| 13:24:26 | gibi | I have to leave early today so I will add unit test and reno to the patch on monday | |
| 13:29:07 | artom | Sure | |
| 13:43:16 | sean-k-mooney | gibi: ack ill review that shortly i just got back form doctors appoinent they were runing 90 mins behind so took longer then planned | |
| 13:44:54 | gibi | thanks | |
| 13:45:04 | gibi | no rush, I will probably look at it only on Monday | |
| 13:45:44 | sean-k-mooney | ack | |
| 13:56:31 | sean-k-mooney | gibi: i have set the public security flag on https://bugs.launchpad.net/nova/+bug/1981813 by the way just to highlight it to the security team | |
| 13:57:15 | sean-k-mooney | it was a public bug downstream so that ship had already sailed when we triaged it | |
| 14:12:21 | opendevreview | Stephen Finucane proposed openstack/nova master: Fix compatibility with jsonschema 4.x https://review.opendev.org/c/openstack/nova/+/849867 | |
| 14:12:21 | opendevreview | Stephen Finucane proposed openstack/nova master: Remove unused requirement https://review.opendev.org/c/openstack/nova/+/850006 | |
| 14:12:37 | stephenfin | sean-k-mooney: Sorry, I missed your ping earlier. Respun with an arbitrary version now ^ | |
| 14:14:30 | sean-k-mooney | cool | |
| 14:14:39 | sean-k-mooney | will look soon | |
| 15:23:45 | opendevreview | Stephen Finucane proposed openstack/nova master: Bump jsonschema minimum to 4.0.0 https://review.opendev.org/c/openstack/nova/+/850021 | |
| 15:55:41 | sean-k-mooney | artom: you are aware that what you sugesed in https://review.opendev.org/c/openstack/nova/+/849104 is not allowed or what you are ment to do right | |
| 15:56:13 | artom | sean-k-mooney, what's not allowed? | |
| 15:56:37 | sean-k-mooney | using _IntegratedTestBase in new regression tests | |
| 15:56:55 | artom | o_O | |
| 15:57:00 | artom | Is that a new thing? | |
| 15:57:06 | sean-k-mooney | the regression tests are explicatly not ment to use the full test infra and we are not ment to use _IntegratedTestBase for new test in genera | |
| 15:57:19 | artom | ... | |
| 15:57:30 | sean-k-mooney | artom: yes its docuemnted in the readme for the regression tests i linked it in the commend inline | |
| 15:57:38 | artom | [artom@zoe nova]$ ag _IntegratedTestBase nova/tests/functional/regressions/ | wc -l | |
| 15:57:38 | artom | 19 | |
| 15:57:49 | sean-k-mooney | yep there are cases where its used | |
| 15:57:59 | sean-k-mooney | but we are trying not to do ti any more | |
| 15:58:16 | artom | Are we? Why? Because backports? | |
| 15:58:42 | sean-k-mooney | because regression tests are ment to be independing of the rest fo the fucntest so that they are not impacted by refacortings | |
| 15:59:10 | sean-k-mooney | it also help with backprots but that is not the main reason | |
| 15:59:33 | artom | I mean https://review.opendev.org/c/openstack/nova/+/812126 | |
| 15:59:45 | artom | That's at the beginning of this year | |
| 15:59:56 | sean-k-mooney | yep i know that should not have used that | |
| 16:00:31 | artom | Like, we can discuss this, and others can correct me if I'm wrong, but this is the first I hear of it, and I wasn't aware it was an established thing that all core reviewers had agreed on. | |
| 16:00:46 | sean-k-mooney | its in the readme | |
| 16:00:52 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/README.rst#writing-regression-tests= | |
| 16:01:09 | artom | And clearly at least *some* people have not read it :) | |
| 16:01:54 | sean-k-mooney | right this is a very long standing policy for that subdirectory | |
| 16:02:03 | artom | Of not reading it? :D | |
| 16:02:08 | sean-k-mooney | it was intoduced when it was created https://github.com/openstack/nova/commit/5fe04c2ee8b721ecdaea4d06de54b4998a0df395 | |
| 16:02:43 | sean-k-mooney | i had condiered asking gibi to rewrite there repoducer to follow it a few minute ago | |
| 16:03:01 | sean-k-mooney | but we dont require all repoduced to be added as freestandign regressions | |
| 16:03:02 | sean-k-mooney | so i did not | |
| 16:03:15 | sean-k-mooney | but if they are in that folder they should be freestanding like that | |
| 16:06:15 | artom | Genuinely news to me. | |
| 16:06:32 | artom | If we want to make that a thing I won't stand in anyone's ways, but I disagree that it's already a thing. | |
| 16:06:44 | sean-k-mooney | it reallly is | |
| 16:06:57 | artom | I mean in that case let's add a hacking rule | |
| 16:07:16 | artom | If it finds _IntegratedTestCase in the regressions folder, pep8 -1 | |
| 16:07:27 | artom | If we're *actually* going to be strict about it | |
| 16:07:34 | sean-k-mooney | artom: the other thing to keep in mind is that stephen had been trying to remvoe _IntegratedTestCase at one point | |
| 16:07:48 | artom | stephenfin's been trying to remove *everything* at one point ;) | |
| 16:07:49 | sean-k-mooney | so we should not be directlly adding new tests that use it as a base | |
| 16:08:02 | sean-k-mooney | right but _IntegratedTestCase is considerd private | |
| 16:08:12 | sean-k-mooney | and new code should not inherit form it directly | |
| 16:08:17 | artom | I feel like facts on the ground contradict that :) | |
| 16:08:20 | sean-k-mooney | its subclasses are fine but not that | |
| 16:09:25 | sean-k-mooney | i think lee used it for conveicne not because it was the most correct way to repoduce | |
| 16:11:06 | sean-k-mooney | we generally try to be pragmatic about this but i disagree that amit shoudl rewrite the test they created | |
| 16:13:01 | sean-k-mooney | ProviderUsageBaseTestCase would be prefible if you require someing other then the mixins for new tests | |
| 16:14:28 | sean-k-mooney | or LibvirtProviderUsageBaseTestCase if you need libvirt | |
| 17:03:21 | opendevreview | Artom Lifshitz proposed openstack/nova master: hacking: forbid _IntegratedTestBase in regression tests https://review.opendev.org/c/openstack/nova/+/850053 | |
| 17:03:34 | artom | sean-k-mooney ^^ Yes, I'm part-trolling :) | |
| 17:03:46 | artom | Also, it doens't actually work, which... is surprisingly annoying? | |
| 17:03:56 | artom | Like, I thought I'd be able to hack it up quicker, so the ego takes a hit | |
| 17:04:26 | sean-k-mooney | its kind fo hard to do sicne its not about _IntegratedTestBase sepcificaly | |
| 17:04:37 | sean-k-mooney | we are alowed to use the mixins | |
| 17:06:00 | artom | Oh, like those will never get refactored :P | |
| 17:06:34 | sean-k-mooney | well they should not the can have methods added but they idaaly would be addditive changes only or mainly | |
| 17:12:49 | sean-k-mooney | im going to call it a day chat to you monday o/ | |
| 17:26:27 | artom | Oh crap that remind me, I'm off next week | |
| #openstack-nova - 2022-07-18 | |||
| 04:28:00 | opendevreview | Merged openstack/nova stable/wallaby: Add missing condition https://review.opendev.org/c/openstack/nova/+/847011 | |
| 05:00:37 | opendevreview | Merged openstack/nova master: Update the file for IPv4-only or IPv6-only network https://review.opendev.org/c/openstack/nova/+/465891 | |
| 05:12:10 | opendevreview | Merged openstack/nova master: etc: Highlight absence of packages from config gen https://review.opendev.org/c/openstack/nova/+/849796 | |
| 07:02:52 | opendevreview | Merged openstack/nova stable/victoria: libvirt: make mdev types name attribute be optional https://review.opendev.org/c/openstack/nova/+/754401 | |
| 07:16:50 | bauzas | good Monday everyone | |
| 07:18:43 | sean-k-mooney | bauzas: o/ | |
| 07:19:25 | sean-k-mooney | im up too early for me to say if its good yet but i should proably go have coffee and then form an opipion | |
| 07:20:07 | bauzas | :) | |
| 07:22:44 | sean-k-mooney | bauzas: we have one that we like the general idea of but wanted more info from the autuhor and were considering giving a freeze exception if they provide that info by the team meeting | |
| 07:23:06 | bauzas | sean-k-mooney: I haven't looked yet at the open specs | |
| 07:23:17 | sean-k-mooney | https://review.opendev.org/c/openstack/nova-specs/+/849488 | |
| 07:23:48 | bauzas | oh I remember having seen it | |
| 07:23:56 | bauzas | it was created by July 11... | |
| 07:24:27 | sean-k-mooney | the rest can all get procedual -2 or abandoned | |
| 07:24:53 | sean-k-mooney | bauzas: its small stephen was even wondering if it shoudl be a specless blueprint before i pointed out they had a spec | |
| 07:25:46 | sean-k-mooney | bauzas: did you enjoy you time in th emountains | |
| 07:27:04 | bauzas | sean-k-mooney: oh yes :) https://twitter.com/matthewdbooth/status/1547252109179080705 | |
| 07:27:56 | sean-k-mooney | :) | |
| 08:04:04 | gibi | good morning | |
| 08:11:26 | sean-k-mooney | o/ | |
| 08:12:17 | sean-k-mooney | bauzas: i reviewed https://review.opendev.org/c/openstack/nova/+/849133 while you were away id allowing space intentional? over all i think this looks good as is | |
| 08:13:21 | bauzas | sean-k-mooney: excellent catch | |
| 08:13:37 | bauzas | this part was just rebased from the previous effort | |
| 08:13:43 | bauzas | I need to reconsider this | |
| 08:13:58 | sean-k-mooney | it proably ok to allow space | |
| 08:14:00 | bauzas | either way, I also need to rebase my work on Uggla's unshelve-to-host API part | |
| 08:14:05 | sean-k-mooney | i jsut would not personally use it | |
| 08:14:19 | bauzas | sean-k-mooney: honestly, I need to consider the RFC | |
| 08:14:38 | sean-k-mooney | ok no rush | |
| 08:15:06 | sean-k-mooney | so are we agreeing on merging the unshelve-to-host change first | |