| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-27 | |||
| 12:03:52 | gibi | ahh, yeah, I ment there is automatic autospec in both lib and stdlib | |
| 12:04:56 | gibi | sean-k-mooney: yeah, if A().foo is already a mocked then that mock's patcher.stop() will revert the change. But the caller doing the A().foo = mock.Mock() does not know if foo is a Mock. So we establish a dangerous pattern | |
| 12:05:37 | gibi | if that pattern is used to other than a mocked field the we will leak global state between tests | |
| 12:06:22 | sean-k-mooney | gibi: right but we would only do that for the double mock case | |
| 12:06:40 | sean-k-mooney | but i agreee in general its bad | |
| 12:06:49 | sean-k-mooney | we have leaked state that way in the past | |
| 12:06:51 | gibi | yes, now, but then the next new dev came look at the code and think that A().foo = mock.Mock() is a generic pattern | |
| 12:07:05 | sean-k-mooney | where a mock was assinged to a stdlib function | |
| 12:07:25 | sean-k-mooney | so that is why i was wondering if we need a nova helper fucniton | |
| 12:07:34 | sean-k-mooney | to basiclaly always do the right thing | |
| 12:07:50 | gibi | but that mean all our mocking need to go through that helper | |
| 12:07:54 | gibi | that will be a massive change | |
| 12:08:19 | sean-k-mooney | perhaps i need to think about what pattern we can actully use that will work | |
| 12:08:36 | gibi | anyhow I'd like to do a non theorethicaly investigation with this | |
| 12:08:40 | sean-k-mooney | perhaps using the mock as a context manager | |
| 12:08:49 | sean-k-mooney | but not sure if that would still fail for the double mocking case | |
| 12:09:04 | gibi | btw this is why I remembered automatic autospeccing https://review.opendev.org/q/message:autospec+project:openstack/nova+status:merged | |
| 12:09:53 | sean-k-mooney | sure but that not automatic | |
| 12:10:06 | sean-k-mooney | that was exiplcitly enabling it and new right | |
| 12:10:11 | gibi | yeah, I missremembered | |
| 12:10:16 | sean-k-mooney | oh ok | |
| 12:10:54 | gibi | I will look at this mock thing tomorrow | |
| 12:11:10 | gibi | now I'm in a middle fighting with reshape + provider generation conflict | |
| 12:12:12 | gibi | I'm in the denial phase where I think my code is correct and reshape is broken :D | |
| 12:12:46 | sean-k-mooney | :) | |
| 12:13:08 | sean-k-mooney | maybe but if thats the case you will probly have to fix it in placment yes | |
| 12:13:17 | sean-k-mooney | so hopefully the issue is in your current code | |
| 12:13:21 | gibi | I know :D | |
| 12:13:24 | sean-k-mooney | that feels like less work | |
| 12:13:25 | gibi | hence "denial" | |
| 12:13:54 | gibi | anyhow I think this is the first case when we both create new RPs and reshape allocations at the same time | |
| 12:14:08 | gibi | from nova | |
| 12:14:55 | gibi | and RP creation is outside of the reshape code | |
| 12:15:02 | sean-k-mooney | if nova has not done it its likely no one else has | |
| 12:15:17 | sean-k-mooney | i dont think neutron or cyborg have used reshapes before | |
| 12:16:28 | gibi | me neither | |
| 12:50:54 | bauzas | I just capture the discussion | |
| 12:51:10 | bauzas | is it because unittest.mock is not on par with our mock lib ? | |
| 12:52:16 | gibi | it is that in py310 unittest.mock introduced a breaking change. The mock lib is a rolling backport of the upstream unittest.mock so at some point this change will appera in the standalone mock lib too | |
| 12:52:19 | kashyap | bauzas: Please put the stable/wallaby backport through: https://review.opendev.org/c/openstack/nova/+/851206 | |
| 12:52:34 | kashyap | gibi ACKed it too; and you've ACKed the upstream main | |
| 12:53:39 | gibi | kashyap: we need to land the backports in order | |
| 12:53:44 | gibi | so we land the yoga first | |
| 12:53:48 | gibi | then xena | |
| 12:53:50 | gibi | then wallaby | |
| 12:54:03 | kashyap | gibi: Oh, yes. I mean to also implicitly look at 'em. :) | |
| 12:54:11 | gibi | ack | |
| 12:54:21 | bauzas | gibi: which breaking change ? | |
| 12:54:53 | gibi | if you mock a function twice, the second mocking attempt will fail | |
| 12:55:05 | gibi | as autospecing a mock is disallowed now | |
| 12:55:48 | bauzas | then we should wait for our mock lib | |
| 12:55:57 | gibi | what? why? | |
| 12:56:19 | gibi | why it is better to get the same breaking change later? | |
| 12:56:20 | bauzas | we should change the related tests first due to this | |
| 12:57:04 | bauzas | gibi: well, once our mock lib would be modified, then the CI wouldn't work and we would see it so we could not accept the new release | |
| 12:57:16 | bauzas | then we could modify the tests | |
| 12:57:23 | bauzas | for this new release | |
| 12:57:32 | bauzas | and then accepting the new release | |
| 12:58:07 | gibi | this is the same thing. If we want to accept py310 it means we need to accept all the breaking changes there. | |
| 12:59:13 | gibi | so we are only debating when to fix those duplicated mocks | |
| 12:59:27 | bauzas | at least we need to modify the only needed tests | |
| 12:59:29 | gibi | and as the mock switch is ready now I think we should fix it now | |
| 12:59:40 | bauzas | before changing to use unittest.mock | |
| 13:00:17 | gibi | why before? we can switch to unittest.mock and then fix the duplicated mock to get the py310 non voting job gren | |
| 13:00:20 | gibi | green again | |
| 13:01:06 | gibi | switching first make py310 job telling us where are the duplicates | |
| 13:01:55 | admin1 | i have nova using local disk , but glance and cinder uses ceph .. when using snapshots, i get broken pipe from nova compute .. is there any special way ceph needs to be configured for snapshots to work ? | |
| 13:12:03 | sean-k-mooney | admin1: no in that config nova should upload the snapshot via the glace http api | |
| 13:12:25 | sean-k-mooney | so it will create a snapshot using qemu-image then upload that disk | |
| 13:19:53 | opendevreview | Sylvain Bauza proposed openstack/nova master: api: Drop generating a keypair and add special chars to naming https://review.opendev.org/c/openstack/nova/+/849133 | |
| 13:20:14 | bauzas | rebased my keypair api microversion due to unshelve_to_host merge ^ | |
| 13:20:24 | bauzas | people can review it | |
| 13:27:56 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/victoria: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851223 | |
| 13:31:38 | gibi | bauzas: +2, do you have an python binding patch and or osc patch with the client side change? | |
| 13:31:45 | bauzas | not yet | |
| 13:31:53 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/victoria: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851223 | |
| 13:32:04 | gibi | Uggla: similar question to you too, do you have a client patch for the unshelve to host change? | |
| 13:32:08 | gibi | bauzas: ack | |
| 13:32:31 | sean-k-mooney | technically we dont need thos to merge the nova change but ya woudl be nice to see them sooner rather then later | |
| 13:32:38 | Uggla | gibi, yes give you the ids in one sec | |
| 13:32:39 | gibi | in the past we said we would at least like to see the client patches proposed (not merged) before we approve a microversion | |
| 13:32:45 | gibi | Uggla: awesome | |
| 13:32:49 | sean-k-mooney | ill be dropping soon but ill see if i have time to review before i do | |
| 13:33:32 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/ussuri: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851224 | |
| 13:34:22 | Uggla | gibi: https://review.opendev.org/c/openstack/python-novaclient/+/831651 and https://review.opendev.org/c/openstack/python-openstackclient/+/831902 | |
| 13:34:33 | gibi | Uggla: thanks | |
| 13:35:03 | Uggla | gibi, there is also a related tempest test: 841088: Tempest test for checking unshelve to host | https://review.opendev.org/c/openstack/tempest/+/841088 | |
| 13:35:16 | gibi | ack | |
| 13:38:26 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/train: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851226 | |
| 13:40:21 | kashyap | bauzas: gibi: If you have the stomach for it today still (other backports): https://review.opendev.org/q/Iec387dcbc49ddb91ebf5cfd188224eaf6021c0e1 | |
| 13:40:41 | bauzas | gibi: kashyap: was working on the novaclient change | |
| 13:40:49 | kashyap | No worries | |
| 13:46:31 | opendevreview | Merged openstack/nova master: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851034 | |
| 14:04:35 | gibi | Uggla: I left feedback in the python-novaclient patch and +1d the osc patch. nice work! | |
| 14:05:14 | Uggla | gibi, ok I'll have a look ASAP. | |
| 14:12:57 | opendevreview | Sylvain Bauza proposed openstack/python-novaclient master: Add support for 2.92 : keypair import mandatory https://review.opendev.org/c/openstack/python-novaclient/+/851231 | |
| 14:13:02 | bauzas | gibi: ^ | |
| 14:13:09 | gibi | bauzas: thanks | |
| 14:13:13 | bauzas | now working on the OSC patch | |
| 14:14:14 | gibi | bauzas: you probably want to put your novaclient patch top of Uggla's | |