| opendevreview | Ghanshyam Maan proposed openstack/nova master: Fix graceful shutdown functional unstable tests https://review.opendev.org/c/openstack/nova/+/1002223 | 00:41 |
|---|---|---|
| opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Add functional tests for TDX capability report https://review.opendev.org/c/openstack/nova/+/998525 | 01:07 |
| opendevreview | Merged openstack/nova master: Stub kernel support check of AMD-SEV in tests https://review.opendev.org/c/openstack/nova/+/999780 | 03:19 |
| opendevreview | Merged openstack/nova master: libvirt: Check only x86_64 arch for Intel TDX https://review.opendev.org/c/openstack/nova/+/1000805 | 05:41 |
| opendevreview | Merged openstack/nova master: Remove unused default argument https://review.opendev.org/c/openstack/nova/+/922073 | 06:27 |
| gibi | antia: TDX landed. Thanks for the contribution! | 07:51 |
| gibi | a bunch of stabilization bug fix landed. I now took https://bugs.launchpad.net/nova/+bug/2165150 as the next to work on | 08:16 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Add functional tests for TDX capability report https://review.opendev.org/c/openstack/nova/+/998525 | 08:16 |
| antia | gibi: Amazing! Thanks for all your effort on this | 08:30 |
| gibi | now that both TDX and SNP is in the tree I will buy a slice of cake after lunch. It was a nice cooperation across timezones, companies, and even vendors. I would like to see more such feature development in the future in openstack | 08:32 |
| gibi | thanks tkajinam antia sean-k-mooney gmaan bauzas and others in the background who are not on IRC to make it happen | 08:33 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Add functional tests for TDX capability report https://review.opendev.org/c/openstack/nova/+/998525 | 09:48 |
| gmaan | \o/ | 12:16 |
| gibi | I think I found a race that is not test only but a real race in the compute(s) https://bugs.launchpad.net/nova/+bug/2165318 | 12:24 |
| opendevreview | Ghanshyam Maan proposed openstack/nova master: Add event.set cleanup in test_graceful_shutdown https://review.opendev.org/c/openstack/nova/+/1002638 | 12:35 |
| gmaan | gibi: ^^ followup for self.addCleanup event.set | 12:38 |
| gibi | thanks | 12:40 |
| opendevreview | Lajos Katona proposed openstack/nova master: libvirt: Preserve queue_size during live migration https://review.opendev.org/c/openstack/nova/+/995841 | 13:25 |
| opendevreview | Merged openstack/nova master: Remove workaround for ServiceAuthTokenWrapper get_endpoint_data() https://review.opendev.org/c/openstack/nova/+/1002540 | 13:52 |
| stephenfin | melwitt: I'd appreciate your eyes on https://review.opendev.org/c/openstack/nova/+/1002359 while you still have ksa know-how fresh in your head 🙏 | 14:29 |
| melwitt | stephenfin: ah yes thanks for reminding. writing it down this time | 14:29 |
| opendevreview | Stephen Finucane proposed openstack/nova master: conf: Deprecated http_retries options https://review.opendev.org/c/openstack/nova/+/1002657 | 14:49 |
| opendevreview | Stephen Finucane proposed openstack/nova master: WIP: network: Simplify client creation https://review.opendev.org/c/openstack/nova/+/1002658 | 14:49 |
| melwitt | gmaan: can I get a sanity check regarding a stable backport proposed? it would change a HTTP 403 to a HTTP 400. I believe this should be ok for stable bc it is remaining a 4xx status still, but wanted to check with you https://review.opendev.org/c/openstack/nova/+/996878 | 15:10 |
| gmaan | melwitt: sure, will check after internal meeting. | 15:11 |
| gmaan | melwitt: thanks for review on gate fixes, this one also ready https://review.opendev.org/c/openstack/nova/+/1002223 | 15:11 |
| melwitt | cool thank you | 15:12 |
| gmaan | melwitt: stephenfin and this one you had reviewed earlier, i need to rebase it due to code refactor on this https://review.opendev.org/c/openstack/nova/+/986477 | 15:12 |
| melwitt | gmaan: thanks for the fixes! I'm very happy for the improvements. thanks I will review that one too | 15:12 |
| gmaan | thanks | 15:12 |
| melwitt | gmaan: regarding this review comment https://review.opendev.org/c/openstack/nova/+/1002223/comment/9997bc1c_41882d81/ doesn't that mean that the _wait_for_service_parameter will not actually wait at all? if the service reports "up" before the first heartbeat, wouldn't this _wait_for_service_parameter just return immediately seeing "up" and not actually wait? | 15:21 |
| opendevreview | Stephen Finucane proposed openstack/nova master: conf: Deprecate http_retries options https://review.opendev.org/c/openstack/nova/+/1002657 | 15:23 |
| opendevreview | Stephen Finucane proposed openstack/nova master: WIP: network: Simplify client creation https://review.opendev.org/c/openstack/nova/+/1002658 | 15:23 |
| gmaan | melwitt: yes, it will immediately return as service is up. one issue i found where it was waiting was when we restart the service. in restart test failed as service did not came as 'up' just after restart | 15:23 |
| gmaan | but yes, after service start, it will be up till first heartbeat is there or service_down_time elapsed | 15:24 |
| melwitt | thanks | 15:25 |
| melwitt | I wondered if it could somehow be a problem for the tests if it doesn't actually wait but I guess not | 15:25 |
| stephenfin | melwitt: I agree that it would be good to document this. As noted on the review, my understanding is that it's okay to change an error response status code but not to add a new code | 15:30 |
| stephenfin | hopefully I have that right | 15:30 |
| melwitt | I think you are right | 15:31 |
| melwitt | I had been thinking of something else where I wonder if it's not cool to go across code "levels" in certain directions. like it's cool to go from 500 => 400 but not to go from 400 => 500 | 15:32 |
| melwitt | and going from a 4xx to a different 4xx stays within a level (in addition to being an error code that is already currently being returned) | 15:33 |
| opendevreview | Clif Houck proposed openstack/nova master: perf(ironic): eliminate O(N²) ProviderTree deepcopy at startup https://review.opendev.org/c/openstack/nova/+/980676 | 15:58 |
| opendevreview | Clif Houck proposed openstack/nova master: Parallelize per-node resource updates https://review.opendev.org/c/openstack/nova/+/980679 | 15:58 |
| sean-k-mooney | we are alwasy allowed ot change a 500 to a 400 if it was previosly broken | 16:13 |
| sean-k-mooney | 400->500 is just a bug | 16:14 |
| sean-k-mooney | that we shoudl fix before intentully intoducing :) | 16:14 |
| sean-k-mooney | https://docs.openstack.org/nova/latest/contributor/microversions.html#id2:~:text=Footnotes | 16:14 |
| sean-k-mooney | ``` | 16:15 |
| sean-k-mooney | When fixing 500 errors that previously caused stack traces, try to map the new error into the existing set of errors that API call could previously return (400 if nothing else is appropriate). Changing the set of allowed status codes from a request is changing the contract, and should be part of a microversion ``` | 16:15 |
| sean-k-mooney | so 500 to any exisitng responce code is fine and 400, 403, 404 and 415 are alwasy allowed per footnote 2 | 16:16 |
| gmaan | melwitt: on 403->400, yes it is all good to change it to existing code. if it is changing to same level but a new code then it is not backward compatible but it that changing code is existing one then it is ok. | 16:33 |
| opendevreview | Merged openstack/nova master: ci: Remove GLANCE_STANDALONE https://review.opendev.org/c/openstack/nova/+/996322 | 16:42 |
| melwitt | gmaan: awesome thanks | 16:44 |
| melwitt | sean-k-mooney: yes I remember we can change from 5xx to any other error. but I was thinking about other "level" changes but there aren't really any good examples ... the example of 4xx => 5xx is obviously not ok | 16:48 |
| sean-k-mooney | not without a microversion correct | 16:51 |
| sean-k-mooney | but we shoudl not really return a 5xx in general | 16:51 |
| melwitt | yeah, I think 5xx is never desired. that's if all other error handling falls through | 16:52 |
| sean-k-mooney | at least not intentionally unless we are signaling a rate limiting issue or propagaging a 5xx we recived form a different service | 16:52 |
| sean-k-mooney | well 500 is what you get when you hit an edge case we didng accoutn for right | 16:52 |
| melwitt | generally I think so | 16:53 |
| opendevreview | Merged openstack/nova master: libvirt: Drop redundant cast to str in python 3 https://review.opendev.org/c/openstack/nova/+/997810 | 17:28 |
| opendevreview | Merged openstack/nova master: libvirt: Fail when file-backed memory is used with reservation https://review.opendev.org/c/openstack/nova/+/906161 | 17:49 |
| opendevreview | Merged openstack/nova master: Fix race between RPC method completion and task tracking updates https://review.opendev.org/c/openstack/nova/+/1002432 | 17:55 |
| opendevreview | Merged openstack/nova master: Add event.set cleanup in test_graceful_shutdown https://review.opendev.org/c/openstack/nova/+/1002638 | 17:55 |
| *** bauzas1 is now known as bauzas | 18:07 | |
| *** Unknown123 is now known as Mike-- | 18:19 | |
| gmaan | melwitt: replied to your question https://review.opendev.org/c/openstack/nova/+/1002223/comment/c5a84643_8a1c0fa7/ | 19:09 |
| gmaan | sorry for being lazy to explain on why we need wait after start_compute. though it is only needed in _restart_compute and not in setUp | 19:09 |
| gmaan | let me know if you prefer to remove it from setUp, i can respin it quickly | 19:10 |
| gmaan | basically there is difference in service status until first heartbeat arrives for start fresh service vs restart existing service. where in former case, service initial status is 'up' and latter one is 'down'. once first heartbeat arrives it is as per the heartbeat and service_down_time | 19:12 |
| melwitt | gmaan: haha thanks. yes that makes sense that it's necessary for the restart. I was confused by your last comment about tests failing without the wait because we were commenting at the setUp location in the code :) | 19:14 |
| gmaan | yeah, i commented that in wrong place. let me remove it from setUp to avoid confusion in future | 19:15 |
| melwitt | I kept thinking about it like, that doesn't make sense at this location. I understand now, you meant at the restart location | 19:15 |
| opendevreview | Ghanshyam Maan proposed openstack/nova master: Fix graceful shutdown functional unstable tests https://review.opendev.org/c/openstack/nova/+/1002223 | 19:25 |
| gmaan | melwitt: ^^ fixed it and also added notes | 19:26 |
| melwitt | cool thanks! | 19:26 |
| *** thelounge55146 is now known as thelounge5514 | 19:43 | |
| *** Unknown123 is now known as Mike-- | 19:43 | |
| opendevreview | Merged openstack/nova master: perf(ironic): eliminate O(N²) ProviderTree deepcopy at startup https://review.opendev.org/c/openstack/nova/+/980676 | 21:33 |
| opendevreview | Merged openstack/nova master: Add shebang to mark bash files https://review.opendev.org/c/openstack/nova/+/995213 | 21:43 |
| *** erlon3 is now known as erlon | 22:18 | |
| *** jlejeune8 is now known as jlejeune | 23:03 | |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!