| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-05-25 | |||
| 13:18:32 | gibi | sean-k-mooney: yes | |
| 13:18:39 | gibi | for me this is the restful put | |
| 13:18:52 | sean-k-mooney | if you do a update of the full resouce yes | |
| 13:18:56 | bauzas | this | |
| 13:19:18 | bauzas | if you just PUT a name, this isn't RESTful | |
| 13:19:37 | gibi | but the current PUT is not partial and I'm not suggesting any partial PUT | |
| 13:19:40 | gibi | either | |
| 13:19:46 | bauzas | and if you PUT a parent uuid which is not None while the resource parent is not None, then it's not RESFul either | |
| 13:20:06 | bauzas | because you depend on the state of the resource | |
| 13:20:23 | bauzas | but we're bikeshedding I guess | |
| 13:20:28 | sean-k-mooney | gibi: right i think bauzas was assuming you wanted to change that | |
| 13:20:43 | sean-k-mooney | bauzas: were you assuming gibi was suggeting using put for a partial update | |
| 13:20:57 | gibi | so today, PUT takes 3 paramters, name, uuid, and parent_uuid. After my change PUT takes the same parameters | |
| 13:21:07 | bauzas | actually, I was wrong | |
| 13:21:11 | gibi | just the value restriciotn of parent_uuid is relaxed | |
| 13:21:12 | sean-k-mooney | gibi: maybe we coudl put json exmaple into the spec to make that clear | |
| 13:21:14 | bauzas | name and uuid aren't optional | |
| 13:21:21 | bauzas | but parent_uuid is | |
| 13:21:37 | gibi | I assume parent_uuid field is not optional, it just allow taking the value of None | |
| 13:21:57 | bauzas | gibi: tbc, I'd be OK with a parent_uuid QS param name | |
| 13:22:05 | sean-k-mooney | gibi: right that is how i think of that too | |
| 13:22:37 | gibi | the API ref says optional, but I'm not sure that it is just due to the microversion dependency | |
| 13:22:39 | bauzas | but I'm not OK with some allow_reparenting name | |
| 13:22:40 | sean-k-mooney | the parent is an intrinic part of the resouce but None is a valid and default value | |
| 13:23:06 | sean-k-mooney | bauzas: you can change the name whenever you like | |
| 13:23:16 | sean-k-mooney | oh sorry | |
| 13:23:19 | sean-k-mooney | misread that | |
| 13:23:37 | sean-k-mooney | bauzas: you ment you are not ok with teh allow_reparenting query arg name | |
| 13:24:24 | sean-k-mooney | bauzas: i would prefer allow_reparenting over force | |
| 13:33:38 | openstackgerrit | Merged openstack/nova-specs master: Set minversion of tox to 3.18.0 https://review.opendev.org/c/openstack/nova-specs/+/791975 | |
| 13:33:39 | sean-k-mooney | gibi: ill get back to the reparenting spec later today. left my toughts on this as comments but i have no stong feeling agaisnt the current query arg approch nesssarly. | |
| 13:33:57 | gibi | bauzas, sean-k-mooney: thanks | |
| 13:46:22 | masterpe | I have placed indexes on the deleted_at column on all the tables on the databases nova, nova_api and nova_cell0, this speeds this nova-manage db archive_deleted_rows with multiple minutes per run. First it was 19 minutes now it is 19 seconds. | |
| 13:46:38 | masterpe | So I think this is a bug. | |
| 13:47:01 | gibi | masterpe: please file a bug. and feel free to propose a patch that adds the index | |
| 13:47:50 | masterpe | Is bugs.launchpad.net/ still used as bug tracker? | |
| 13:47:58 | gibi | for nova, yes | |
| 13:48:12 | gibi | https://bugs.launchpad.net/nova/+filebug | |
| 13:49:18 | gibi | thank you for reporting | |
| 13:49:23 | sean-k-mooney | masterpe: sound like a quick fix. althoguh we dont always index everythin that shoudl be indexed by default | |
| 13:50:18 | sean-k-mooney | deleted should already be indexed yes | |
| 13:50:34 | sean-k-mooney | deleted_at woudl only be imporant if you are using --before | |
| 13:50:48 | sean-k-mooney | i assume we just missed that usage chagne wehn we added --before | |
| 13:51:30 | sean-k-mooney | prior to that deleted_at would be very rarely used in queired as a column we filtered on | |
| 13:52:19 | sean-k-mooney | i think the soft delete code would have been the only thing that checked it really bar a direct user request for deleted instances | |
| 13:59:47 | masterpe | sean-k-mooney: https://bugs.launchpad.net/nova/+bug/1929563 | |
| 13:59:49 | openstack | Launchpad bug 1929563 in OpenStack Compute (nova) "Missing database index deleted_at column" [Undecided,New] | |
| 14:00:36 | masterpe | Do I only need to add the new indexes to ./db/sqlalchemy/models.py ? | |
| 14:02:53 | sean-k-mooney | masterpe: you also need to add a migration to add the indexs i think | |
| 14:03:11 | masterpe | I need to remember how to make commits, so probably it is easyer one someone else does that ;) | |
| 14:04:05 | sean-k-mooney | well if you want to start with the model change then you could | |
| 14:04:39 | sean-k-mooney | and we could pick it up but stephenfin is also looking at swaping out sqlalcamey_migrate for alembic this cycle | |
| 14:04:50 | sean-k-mooney | so how you do that will cahnge soon ish | |
| 14:05:36 | masterpe | That will get into the master branch and not Train ? | |
| 14:06:02 | gibi | bauzas: you are right, PUT /resource_providers/{uuid} is not RESTFul as it allows partial update today. This is very unfortunate :/ | |
| 14:06:53 | gibi | the parent_uuid not need to be passed, and the uuid field is not part of the body just part of the url | |
| 14:07:52 | gibi | the current PUT is very pretty close to PATCH now | |
| 14:23:26 | gibi | stephenfin: we need you on https://review.opendev.org/c/openstack/nova-specs/+/783827 | |
| 14:23:29 | gibi | :0 | |
| 14:23:55 | stephenfin | I'm needed? How wonderful | |
| 14:24:00 | stephenfin | * stephenfin looks | |
| 14:24:22 | gibi | it is your spec where sean-k-mooney has some comments making it pending | |
| 14:36:11 | sean-k-mooney | stephenfin: my main question is do we need OS-EXT-SRV-ATTR:hostname anymore and shoudl we just have hostname instead | |
| 14:36:57 | stephenfin | ah, that point | |
| 14:37:03 | stephenfin | I did see that and thought I had replied | |
| 14:37:54 | stephenfin | I'm easy. Changing it is more work, both on the server side (code and docs) and client (SDK, OSC, novaclient) side | |
| 14:37:55 | sean-k-mooney | we could adress that point in a follow up which is why im currently +1 | |
| 14:38:15 | stephenfin | but on the other hand, it's certainly a saner response | |
| 14:38:17 | sean-k-mooney | it would be but i think it would be a nice UX improvemnt | |
| 14:38:28 | sean-k-mooney | changing it in the futrue woudl need another micorversion bump | |
| 14:38:51 | sean-k-mooney | so basically i we ever want to change it i think it would be nice to do it now ihn this change | |
| 14:39:10 | stephenfin | what about the rest of those prefixed options though? | |
| 14:39:18 | stephenfin | if we do one, shouldn't we do them all | |
| 14:39:20 | stephenfin | ? | |
| 14:39:30 | sean-k-mooney | well im open to that also | |
| 14:40:22 | sean-k-mooney | im ok wiht elevating my +1 to a +w and we can dicuss that in a followup patch? although it really was just hostname that i wanted to chagne in this case | |
| 14:40:35 | sean-k-mooney | some of the other prefixed fiels are admin only | |
| 14:41:07 | sean-k-mooney | so im not really sure it there is merrit in renaming those form a UX point of view | |
| 14:44:08 | stephenfin | I would be tempted to do all or nothing, personally | |
| 14:44:29 | sean-k-mooney | let me look at the list. not sure what the otehrs are fully | |
| 14:44:41 | sean-k-mooney | the prefix was form when we had extentions | |
| 14:44:52 | gibi | is this prefixing comes from the time when we had api plugins? | |
| 14:44:53 | sean-k-mooney | to show that they were optional and could not be replied on to be in all clouds | |
| 14:44:54 | gmann | sean-k-mooney: stephenfin left comment there. agree to improve the name | |
| 14:45:03 | sean-k-mooney | gibi: yes | |
| 14:45:15 | gibi | so they can be relied on now | |
| 14:45:20 | sean-k-mooney | yep | |
| 14:45:22 | gibi | so we can drop all the prefix :) | |
| 14:45:32 | gibi | lets do that in a separate microversion. :) | |
| 14:45:35 | sean-k-mooney | we can. that sound like a seperate mini spec so | |
| 14:45:38 | gmann | in past we left these prefix as it is not worth to do with all prefix rename but as in some other microvesion change we should definitely improver them | |
| 14:45:58 | gmann | gibi: we abandon that idea in past | |
| 14:46:02 | gmann | let me check the spec | |
| 14:46:13 | gibi | bottom line, I'm OK with stephenfin's spec as is, and we can have a separate discussion if and when we drop all the prefixes | |
| 14:46:39 | stephenfin | yes, I do think we should treat this separately. Microversions are (relatively) cheap | |
| 14:47:01 | sean-k-mooney | ok in that case ill upgrade my +1 to +w and refrence this irc log | |
| 14:47:16 | sean-k-mooney | unless gibi gmann you want me to wait? | |
| 14:47:30 | gibi | sean-k-mooney: go for it | |
| 14:47:30 | openstackgerrit | masterpe proposed openstack/nova master: WIP: Fixes missing indexes on column deleted_at at the different tables. https://review.opendev.org/c/openstack/nova/+/792989 | |
| 14:47:40 | gibi | but wait for gmann | |