| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-13 | |||
| 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 | |
| 12:40:22 | auniyal | is there anything else I should look for ? | |
| 12:41:22 | auniyal | bauzas, sean-k-mooney ^ | |
| 12:49:11 | sean-k-mooney | its tags not tag | |
| 12:49:31 | sean-k-mooney | well there are two ways | |
| 12:49:38 | sean-k-mooney | what eactully are you executing | |
| 12:50:00 | sean-k-mooney | you can add a tag by doing a put to /servers/{server_id}/tags/{tag} | |
| 12:50:18 | sean-k-mooney | or you do a put to /servers/{server_id}/tags with {tags": ["tag1", "tag2"]} as the boday | |
| 12:51:29 | sean-k-mooney | https://docs.openstack.org/api-ref/compute/#server-tags-servers-tags | |
| 12:52:04 | sean-k-mooney | if your doing this as part of server create its also tags not tag | |
| 13:06:06 | auniyal | I am adding tag while creating server - https://paste.opendev.org/show/bJBibgysdZSdO5iMAZEg/ | |
| 13:06:28 | sean-k-mooney | that not adding a tag to a server | |
| 13:06:33 | sean-k-mooney | thats addign a tag to a network | |
| 13:06:43 | sean-k-mooney | its not the same thing | |
| 13:06:50 | auniyal | ack | |
| 13:07:03 | auniyal | so I am trying device tagging | |
| 13:07:22 | auniyal | once device is tagged, I'll verify it in metadata.json | |
| 13:07:31 | sean-k-mooney | per the api docs | |
| 13:07:33 | sean-k-mooney | A bug has caused the tag attribute to no longer be accepted starting with version 2.37. Therefore, network interfaces could only be tagged in versions 2.32 to 2.36 inclusively. Version 2.42 has restored the tag attribute. | |
| 13:08:04 | sean-k-mooney | so in the network you call it tag | |
| 13:08:15 | sean-k-mooney | and your using 2.43 | |
| 13:08:22 | sean-k-mooney | so that shoudl be fixed | |
| 13:08:24 | auniyal | yes, 2.43 | |
| 13:09:37 | sean-k-mooney | so https://paste.opendev.org/show/bE1IV21v1cdSzRKNWKrw/ | |
| 13:09:40 | sean-k-mooney | should be valid | |
| 13:09:48 | sean-k-mooney | just s/tags/tag/ | |
| 13:11:48 | sean-k-mooney | https://github.com/openstack/nova/commit/e80e2511cf825671a479053cc8d41463aab1caaa | |
| 13:11:52 | sean-k-mooney | that is the change that fixed it | |
| 13:12:05 | auniyal | this is full what I have right now - https://paste.opendev.org/show/bymbRPlBXG3nmY1Nm4zy/ | |
| 13:13:03 | sean-k-mooney | this is the api sample test that validates it https://github.com/openstack/nova/blob/master/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl | |
| 13:14:34 | sean-k-mooney | you do not need admin_api=ture by the way | |
| 13:15:05 | auniyal | ack | |
| 13:15:39 | sean-k-mooney | so i think the problem is | |
| 13:15:42 | sean-k-mooney | self.api = self.useFixture( | |
| 13:15:44 | sean-k-mooney | nova_fixtures.OSAPIFixture(api_version='v2.1')).api | |
| 13:16:02 | sean-k-mooney | you are seting api_version but i dont think that will work as you expect | |
| 13:17:47 | bauzas | sorry auniyal can't help, today is the last day before RC1 | |
| 13:18:03 | bauzas | IMHO you need to set the microversion correctly | |
| 13:18:13 | auniyal | so yeah, I tried this nova_fixtures.OSAPIFixture(api_version='v2.42')) and got https://paste.opendev.org/show/bHOTRcnwwpPb1lP8BbH5/ | |
| 13:18:31 | bauzas | that's not how you set the microversion | |
| 13:18:34 | sean-k-mooney | ya so when we set version in teh class like that | |
| 13:18:39 | bauzas | please look at other tests | |