| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-05-25 | |||
| 13:11:59 | sean-k-mooney | bauzas: if we were to have a subresouce i actully woudl go with PUT /resource-providers/{uuid}/uuid/{new uuid} | |
| 13:11:59 | bauzas | gibi: hence my +1 :) | |
| 13:12:22 | bauzas | gibi: I don't wanna hold on this, but I don't appreciate the QS param outcome | |
| 13:13:01 | sean-k-mooney | sorry | |
| 13:13:26 | sean-k-mooney | put /resource-providers/{uuid}/parent_uuid/{parent} | |
| 13:13:34 | sean-k-mooney | with an empty body | |
| 13:13:57 | sean-k-mooney | that basically woudl work like patch without using it | |
| 13:14:42 | bauzas | this sounds quite good to me | |
| 13:14:51 | bauzas | but again, I'm just one | |
| 13:14:57 | sean-k-mooney | you also have to expressly opt into that url | |
| 13:16:09 | gibi | bauzas: sorry I don't get why the current PUT is not RESTful | |
| 13:16:16 | gibi | you have to PUT all the attributes | |
| 13:16:26 | gibi | that you POSTed before | |
| 13:16:35 | bauzas | I guess the universe entropy is probably smaller than a discussion of 3 engineers about specing an API endpoint | |
| 13:17:01 | bauzas | count a 4th and you'll wait for the big crunch | |
| 13:17:53 | bauzas | gibi: it's not RESTful in the sense you can't update all the attributes as of now | |
| 13:18:22 | gibi | bauzas: you have restriction of the value of a field yes, but you have to still list all the fields in the PUT | |
| 13:18:24 | sean-k-mooney | bauzas: i think gibi was saying you would get teh current state update teh parent field and then put back the updated state | |
| 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? | |