| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-13 | |||
| 09:47:30 | sean-k-mooney | zigo: in fact the 1.4 bridge release is them going well out of there way to provide a smooth upgrade path | |
| 09:48:01 | WJeffs | Hey all, is anyone running AMD genoa cpus, that I could query some stuff to them? | |
| 09:48:06 | zigo | Well, there should be no need for an upgrade path, there should be backward compat forever, that's the point I'm making. | |
| 09:48:38 | sean-k-mooney | zigo: right which as a software maintainer you hsould knwo woudl be unresable ot continue in a supprot way | |
| 09:48:48 | sean-k-mooney | he coudl freeze the old apis | |
| 09:49:02 | sean-k-mooney | but there were fundemetal changes to the core | |
| 09:49:14 | sean-k-mooney | to use and adpat to new python changes | |
| 09:49:25 | zigo | How do you explain the Linux kernel does it the proper way then? | |
| 09:49:29 | sean-k-mooney | and to solve some correctness issues that needed the api to change | |
| 09:49:58 | zigo | And the kernel is a *WAY* bigger and complicated... | |
| 09:50:01 | sean-k-mooney | zigo: by letting the old api bit rot | |
| 09:50:25 | zigo | Well, the current situation shows it was the way to go... :) | |
| 09:51:01 | sean-k-mooney | im not really sure it does | |
| 09:51:16 | sean-k-mooney | we are not going to be droping 1.4 support in openstack for some time | |
| 09:51:56 | sean-k-mooney | and i dont think zzzeek plans ot dicontinue it for some tiem either | |
| 09:52:08 | sean-k-mooney | yes the fact breakign changes are required is unfortunet | |
| 09:52:24 | sean-k-mooney | that does not mean we shoudl never do them just in frequesntly when its the best option | |
| 09:53:23 | zigo | Don't get me wrong, I do love SQLAlchemy, though in the past, every upgrade to a minor version has broke all of OpenStack. The switch from 1.4 to 2.x is kind of very painful, as we're discussing. Well, that's *very* bad practice that zzzeek should learn to *not* reproduce ever again. | |
| 09:54:07 | sean-k-mooney | its actully not that painful to adapt too | |
| 09:54:16 | sean-k-mooney | but it was not a priorty for most of the maintainers | |
| 09:54:33 | zigo | But the old code should still be working with the new version ... | |
| 09:54:33 | sean-k-mooney | only a few have wanted to spend time on it and done most of the work | |
| 09:54:53 | sean-k-mooney | it is in 1.4 which is a long term supprot branch to make it work | |
| 09:55:01 | zigo | Doesn't work for me... | |
| 09:55:07 | sean-k-mooney | zigo: to be clear some of the api had diffent behavior on diffent backend due to bugs | |
| 09:55:11 | zigo | Only a single version will live in the distro, as we discussed... | |
| 09:55:15 | sean-k-mooney | that coudl not be fixed without breakign changes | |
| 09:55:22 | zigo | Ok. | |
| 09:55:34 | zigo | :) | |
| 09:55:40 | zigo | This makes a lot more sence then. | |
| 09:55:56 | sean-k-mooney | its becasue theere is no one SQL | |
| 09:56:08 | sean-k-mooney | thers is TSQL PSQL and what ever mariadb calls it | |
| 09:56:15 | zigo | Maybe zzzeek can help fixing Heat and oslo.db? | |
| 09:56:29 | sean-k-mooney | oslo.db i think basically works | |
| 09:56:39 | sean-k-mooney | heat im not sure about | |
| 09:56:59 | zigo | Oh, btw, should I revert my packaging to oslo.db 12.3.2 ? | |
| 09:57:24 | sean-k-mooney | stephenfin: ^ | |
| 09:57:28 | zigo | (ie: leave 14.0.0 in Experimental, and just bump to 12.3.2 in unstable...) | |
| 09:58:31 | zigo | Anyways thanks a lot for discussing the mater with me, that's very helpful. | |
| 09:58:45 | zigo | At least, I know what's going on. | |
| 09:58:50 | sean-k-mooney | https://review.opendev.org/q/topic:sqlalchemy-20+status:open | |
| 09:58:59 | sean-k-mooney | that is the topic wiht all the pending packages | |
| 09:59:13 | sean-k-mooney | which unfortunelly still has nova and placement in it | |
| 10:09:30 | zigo | stephenfin: Most patches are from you, so I have to thank you as well ! :) | |
| 10:10:15 | sean-k-mooney | yep stephenfin went out of there way to try and update proejct that did nto have peopel steping up to do it | |
| 10:10:32 | sean-k-mooney | unfortuetly that still needs that projects core team to review it | |
| 10:25:27 | bauzas | sean-k-mooney: again, please understand that we have review priorities | |
| 10:25:42 | bauzas | and sqla 2.0 isn't really a prio | |
| 10:26:37 | sean-k-mooney | bauzas: it really was ment to be | |
| 10:26:48 | sean-k-mooney | and i find the fact you didnt consider it to be one a probelm | |
| 10:27:41 | sean-k-mooney | it means we are not comunicating what our priorites are as a team properly | |
| 10:44:32 | stephenfin | bauzas: I struggle to see how adapting to the new major version of a critical library is anything but a priority, especially when it's being treated as such by every other project team | |
| 11:53:05 | bauzas | sorry was at lunch | |
| 11:53:32 | bauzas | I don't want to argue about why we didn't had time to review yet the series, but I'm just trying to merge it before RC1 | |
| 11:54:00 | bauzas | so, please understand my concerns and the fact that we still also need to review other changes | |
| 12:08:16 | bauzas | stephenfin: will you have time to upload a new PS due to my comment ? | |
| 12:09:21 | stephenfin | I already did, but it's crashing and burning | |
| 12:09:25 | stephenfin | https://review.opendev.org/c/openstack/nova/+/860829/3 | |
| 12:11:10 | sean-k-mooney | that looks like the change you made is actully the problem | |
| 12:11:17 | stephenfin | yup | |
| 12:11:17 | sean-k-mooney | and the previous version was likely correct | |
| 12:11:43 | stephenfin | I did test with another reproducer locally but I must have missed something | |
| 12:12:21 | sean-k-mooney | well its actully failing in tempest too | |
| 12:12:32 | sean-k-mooney | so its not just a test artifact | |
| 12:13:16 | sean-k-mooney | sqlalchemy.exc.InvalidRequestError: One or more mappers failed to initialize - can't proceed with initialization of other mappers. Triggering mapper: 'mapped class BlockDeviceMapping->block_device_mapping'. Original exception was: Instance.block_device_mapping and back-reference BlockDeviceMapping.instance are both of the same direction symbol('MANYTOONE'). Did you mean to | |
| 12:13:18 | sean-k-mooney | set remote_side on the many-to-one side ? | |
| 12:14:06 | bauzas | that's why I want to be on par with the existing | |
| 12:14:33 | bauzas | I'm very afraid of any performance hit we may introduce if we wrongly do | |
| 12:14:44 | sean-k-mooney | it was workign before he added the extra changes | |
| 12:14:51 | bauzas | the problem here is that we don't know if the other-way relationship is needed | |
| 12:15:31 | sean-k-mooney | you mean even though it passses tempest, unit test, functional test adn api test | |
| 12:15:39 | sean-k-mooney | without this change | |
| 12:15:56 | sean-k-mooney | you still think we dont actuly have a stong enough indication that its not used | |
| 12:17:01 | sean-k-mooney | the revers relateion would be lookign up instance by a security group | |
| 12:17:27 | sean-k-mooney | https://docs.openstack.org/api-ref/compute/#list-security-groups-by-server | |
| 12:17:47 | sean-k-mooney | the proxy api only allows you to look up security groups of an instnace | |
| 12:18:04 | sean-k-mooney | so we have no public api for the reverse coralation | |
| 12:18:57 | opendevreview | Stephen Finucane proposed openstack/nova master: objects: Stop fetching from security_groups table https://review.opendev.org/c/openstack/nova/+/860850 | |
| 12:18:57 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Replace use of backref https://review.opendev.org/c/openstack/nova/+/860829 | |
| 12:18:58 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove unused relationships https://review.opendev.org/c/openstack/nova/+/894635 | |
| 12:21:18 | bauzas | sean-k-mooney: yeah, while it was difficult to find whether we use instance.bdm, I just guess that instance.securit_group is way less largely used :) | |
| 12:21:55 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/api.py#L2992-L3228 | |
| 12:22:23 | sean-k-mooney | looking at the code there is nothign that woudl be trying to look up instance by the security group | |
| 12:22:45 | sean-k-mooney | the closest thing is security_group_in_use | |
| 12:22:56 | sean-k-mooney | and that jsut directly uses the SecurityGroupInstanceAssociation table | |
| 12:25:00 | sean-k-mooney | bauzas: anyway looks like stephen removed foreign_keys=uuid, | |
| 12:25:07 | sean-k-mooney | so that presumabnle fixes it locally? | |
| 12:25:15 | stephenfin | yup | |
| 12:25:39 | sean-k-mooney | but honestly i woudl have been much happer merging the older version before we added this. i dont want stephen to respien to drop it | |
| 12:25:44 | stephenfin | shouldn't have been there since there's no foreign key column on that side (many-to-one) | |
| 12:25:45 | sean-k-mooney | but i dont like adding dead code | |
| 12:28:07 | bauzas | the code is already dead | |
| 12:28:46 | bauzas | stephenfin explained me that backref implicetely creates a return relationship | |
| 12:28:58 | bauzas | so we already have this, silently | |
| 12:29:21 | bauzas | some "code" may use this implicit relationship for getting the values | |
| 12:29:30 | bauzas | like, instance.bdm | |
| 12:29:53 | bauzas | fortunately, we have nova objects that are the DB facade | |
| 12:39:43 | auniyal | while writing a functional test for server having tags I am getting this error: https://paste.opendev.org/show/bM4cijMNqvGY70FuEDFG/ | |
| 12:39:55 | auniyal | I have set api_version = '2.43' | |
| 12:40:05 | auniyal | because its required in CLI | |