| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-12-15 | |||
| 14:11:44 | gibi | gmann: thanks for following that | |
| 14:12:17 | gmann | gibi: I will update the l-c fix comments after QA office hour. | |
| 14:12:26 | gibi | gmann: cool | |
| 14:14:01 | sean-k-mooney | gibi: did you see my ping yesterday | |
| 14:14:55 | sean-k-mooney | oh you did | |
| 14:15:08 | sean-k-mooney | it still might be worht a try setting the timeout | |
| 14:26:48 | gibi | sean-k-mooney: you want get the test to time out if it slow? or you want to prevent the test to time out when it is slow? | |
| 14:28:47 | sean-k-mooney | manila just set a 5 minute timeout to workaround slow nodes | |
| 14:28:59 | sean-k-mooney | vs the default which i think is 60 seconds | |
| 14:30:25 | gibi | sean-k-mooney: we don't have per test timeout I saw these test run and pass after 300 seconds on the gate | |
| 14:30:57 | sean-k-mooney | thats what the manila patch added | |
| 14:31:17 | gibi | hm, I saw these in the nova test suite | |
| 14:31:37 | sean-k-mooney | https://review.opendev.org/c/openstack/manila/+/291397/ | |
| 14:33:27 | gibi | sean-k-mooney: yeah, I saw this patch yesterday, what I say is that in nova we don't have a per test case timeout, and also I see that in nova these tests run for a long time (both in case of failing or passing) | |
| 14:36:00 | gibi | so I don't see how the extra timeout for these tests would help | |
| 14:37:02 | gibi | logstash don't want to help me know to show some examples | |
| 14:37:23 | gibi | I only found one: TestNovaAPIMigrationsWalkMySQL.test_walk_versions [192.038984s] ... FAILED | |
| 14:37:39 | gibi | TestNovaAPIMigrationsWalkPostgreSQL.test_walk_versions [43.027941s] | |
| 14:41:52 | sean-k-mooney | ah ok so its likely not a timeout issue | |
| 14:42:32 | sean-k-mooney | i did see a timeout in one of the logs | |
| 14:42:39 | sean-k-mooney | but i asumed that was a test timeout | |
| 14:43:30 | gibi | sean-k-mooney: link me the timeout case then I can double check that | |
| 14:44:22 | gibi | I see them failing after random amont of time. the only time sensitvity I can see is that when it fails then the execution is slower then when it passes | |
| 14:44:30 | gibi | but I saw slow passing executions too | |
| 14:44:44 | gibi | I just haven't seen a fast failing execution | |
| 14:54:24 | openstackgerrit | Ghanshyam proposed openstack/nova stable/rocky: DNM: testing gate https://review.opendev.org/c/openstack/nova/+/767027 | |
| 14:58:46 | lyarwood | gibi: https://review.opendev.org/c/openstack/tempest/+/767165 btw, should resolve this. | |
| 14:58:52 | lyarwood | gibi: did you write an ER query for this btw? | |
| 14:59:23 | openstackgerrit | Ghanshyam proposed openstack/placement master: Fix l-c job and move to latest hacking 4.0.0 https://review.opendev.org/c/openstack/placement/+/766994 | |
| 14:59:27 | gibi | lyarwood: I haven't as I was not able to find the root cause | |
| 14:59:38 | lyarwood | k np | |
| 14:59:38 | gibi | lyarwood: and I agree that as we have the fix the ER query is less important now | |
| 15:01:33 | gibi | lyarwood: thanks for the tempest fix | |
| 15:01:53 | gibi | I might ping in the future with cinder timeouts :) | |
| 15:03:07 | gibi | I might ping _you_ :) | |
| 15:05:28 | lyarwood | gibi: always happy to help, even if it debugging cinder timeouts ;) | |
| 15:05:34 | lyarwood | it is* | |
| 15:06:19 | gibi | honestly, I'm so lost in cinder that I alway get stuck with these failures, give up and go do something else :) | |
| 15:08:56 | lyarwood | gibi: I'm the same with Neutron and PCI stuff tbh | |
| 15:09:06 | lyarwood | actually not the PCI stuff anymore | |
| 15:09:11 | gibi | lyarwood: you can ping me with PCI stuff if needed | |
| 15:09:13 | gibi | ohh | |
| 15:09:14 | gibi | bumer | |
| 15:09:17 | lyarwood | but the Neutron flows still confuse the hell out of me | |
| 15:09:30 | gibi | I have nothing to offer :D | |
| 15:10:09 | bauzas | hmmm, TIL I learned that AZ is mandatory even if the doc says it's optional https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id70 | |
| 15:10:14 | bauzas | gibi: others ^ | |
| 15:11:01 | bauzas | https://docs.openstack.org/api-ref/compute/?expanded=unshelve-restore-shelved-server-unshelve-action-detail#unshelve-restore-shelved-server-unshelve-action | |
| 15:11:14 | bauzas | availability_zone (Optional) | |
| 15:11:14 | bauzas | body | |
| 15:11:14 | bauzas | ||
| 15:11:14 | bauzas | string | |
| 15:11:15 | bauzas | ||
| 15:11:16 | bauzas | The availability zone name. Specifying an availability zone is only allowed when the server status is SHELVED_OFFLOADED otherwise a 409 HTTPConflict response is returned. | |
| 15:11:17 | bauzas | New in version 2.77 | |
| 15:11:22 | bauzas | what the heck it is | |
| 15:12:26 | bauzas | so, basically, if you use the latest microversion, we break you | |
| 15:12:35 | bauzas | you need to pass a specific AZ | |
| 15:12:39 | bauzas | when unshelving | |
| 15:13:34 | lyarwood | https://github.com/openstack/nova/blob/240ee3091c5ec458753983afa90e3a0cc11dc322/nova/api/openstack/compute/shelve.py#L95-L98 | |
| 15:13:37 | gibi | bauzas: is it an api ref doc bug or a code bug? | |
| 15:13:45 | lyarwood | so wouldn't {'unshelve': null} still work? | |
| 15:14:15 | bauzas | lyarwood: no | |
| 15:14:22 | bauzas | see https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/shelve.py#L31 | |
| 15:14:35 | bauzas | gibi: well, the doc says it's an optional field | |
| 15:14:53 | bauzas | and honestly, I don't know why it should be a mandatory field | |
| 15:15:21 | bauzas | I'm a bit afraid we ask AZs everytime people want to unshelve | |
| 15:15:25 | gibi | bauzas: would it be defaulted to 'nova' if not provided? | |
| 15:15:26 | lyarwood | I think {'unshelve': null} still works looking at the API code at least | |
| 15:15:41 | bauzas | lyarwood: no because of the schema | |
| 15:15:51 | bauzas | lyarwood: and I tested it | |
| 15:16:24 | lyarwood | huh it lists null as an acceptable type above | |
| 15:16:31 | gibi | hm, maybe we made the same too strict then, the code works without az provided | |
| 15:16:32 | lyarwood | kk if you've tested it then | |
| 15:16:52 | gibi | ohh, null is special cased | |
| 15:16:57 | bauzas | gibi: lyarwood: http://paste.openstack.org/show/801061/ | |
| 15:17:22 | gibi | so unshelve: null OK but unshelve:{} is not | |
| 15:17:32 | bauzas | anyway, I need to get my kids | |
| 15:17:47 | lyarwood | Value: {} | |
| 15:17:57 | lyarwood | ack np | |
| 15:18:14 | gibi | I vaguely remember other cases when we disallow {} for a server action | |
| 15:23:48 | gibi | yepp, we have similar construct for lock, migrate and rescue, but they are not the same as they not require anything in {}. So it is interesting why we deviated from this pattern in case of unshelve | |
| 15:27:19 | gibi | gmann: I still think we should not try to satify both W503 and W504 at the same time just for the sake of the rules as it does not promote a good code style in my eyes | |
| 15:30:11 | gmann | gibi: if we make all operator in same line with bracket '(' then W503 is automatically solved. that give more consistency also. | |
| 15:30:30 | gmann | I mean fixing it in that consistent way automatically solve the both | |
| 15:32:12 | gibi | gmann: as far as I see you wrapped only the parts of the condition into and extra () that is over the line length | |
| 15:32:50 | gibi | (amount_needed < min_unit or amount_needed > max_unit or ( | |
| 15:32:51 | gibi | amount_needed % step_size != 0)) | |
| 15:32:54 | gibi | eg ^^ | |
| 15:33:10 | gibi | here it is strange why we have an extra () only for the last or but not the other ors | |
| 15:34:00 | gibi | I still think that we should not try to satisfy both W504 and W503 at the same time. we should decide to wrap the line before the operatr (504) or after the operator (503) | |
| 15:40:55 | openstackgerrit | Matteo Sposato proposed openstack/nova master: Refactoring of functional.regression.test_bug_1702454 https://review.opendev.org/c/openstack/nova/+/765997 | |
| 15:45:17 | openstackgerrit | Matteo Sposato proposed openstack/nova master: Refactoring of functional.regression.test_bug_1702454 https://review.opendev.org/c/openstack/nova/+/765997 | |
| 15:45:18 | openstackgerrit | Matteo Sposato proposed openstack/nova master: Functional tests remmoved direct post call https://review.opendev.org/c/openstack/nova/+/766068 | |
| 15:47:53 | openstackgerrit | Matteo Sposato proposed openstack/nova master: Functional tests removed direct post call https://review.opendev.org/c/openstack/nova/+/766068 | |
| 15:53:53 | gmann | gibi: ok. so doing in nova way and ignore W504 ? | |
| 15:55:01 | gibi | gmann: that would give us the consistency with nova too. So yes. If I would start a new project then I might use W504 instead and keep the binary operator at the beginnig of the line as that feels like the more imporant information. | |
| 15:55:48 | gmann | gibi: yeah, i think consistency is imp for easy maintenance. will update the patch. | |
| 15:58:16 | gibi | gmann: thanks, I hope I did not sound too demanding above | |
| 15:58:59 | gmann | gibi: no, it would take much time, i think less things to fix may be :) | |