| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-05-25 | |||
| 12:36:05 | sean-k-mooney | mainly grammer nits/word chocices so far nothing technical | |
| 12:36:09 | bauzas | sean-k-mooney: okay, then we could be discussing about this spec once you look at it | |
| 12:36:46 | sean-k-mooney | actully just got to a technical point | |
| 12:36:58 | sean-k-mooney | i do not belive we should limit the resouce class | |
| 12:37:13 | sean-k-mooney | and i do not belive we should have a singel generic mdev class | |
| 12:37:41 | sean-k-mooney | we should support Custom_ resouce classes | |
| 12:37:54 | sean-k-mooney | im undecieded if we shoudl supprot standard ones or not | |
| 12:38:25 | masterpe | But I see that "nova-manage db archive_deleted_rows" command uses a filter on the deleted_at column when using the --before option. But on that column is no index. | |
| 12:40:00 | bauzas | sean-k-mooney: well, then we would need to ask the operator to provide which custom RC they'd like to get | |
| 12:40:13 | sean-k-mooney | bauzas: yes | |
| 12:40:36 | sean-k-mooney | so must remove choices form mdev_class | |
| 12:40:45 | sean-k-mooney | and allow it to be just a sting | |
| 12:41:17 | sean-k-mooney | we can desided to limit it in python to Custom_ if we want too or not | |
| 12:41:33 | gibi | bauzas: no worries. I'm on a call, but I will try to multitask :) | |
| 12:53:58 | admin0 | lyarwood, do we save any version info or migration info in the nova database .. i have a nova database which is fairly old ..and i need to figure out if its from newton, ocata or pike | |
| 12:58:34 | bauzas | gibi: I soft+1d again your PUT RP spec | |
| 12:58:43 | bauzas | gibi: with another API alternative | |
| 12:59:12 | bauzas | in my mind, QS parameters are just another way to pass attributes without using data | |
| 12:59:36 | bauzas | so, generally those params should be identical to the resource attributes | |
| 12:59:50 | gibi | QS are not part of the object representation, while the body is. So whatever we add to the body we make it part of the object as per REST | |
| 13:00:08 | bauzas | gibi: I proposed to use a sub-resource for this | |
| 13:00:37 | bauzas | /force is maybe a bad name | |
| 13:01:23 | gibi | resource_providers/{uuid}/force feels like an action API that is also not that restful | |
| 13:02:21 | sean-k-mooney | bauzas: ok just pushed my comments on your spec | |
| 13:02:25 | gibi | bauzas: so you feel that we cannot add param to the QS that is not part of the body? | |
| 13:02:59 | sean-k-mooney | admin0: i belive we do have a version for the schema yes | |
| 13:03:07 | sean-k-mooney | admin0: i think there is a nova manage command to get it | |
| 13:03:31 | gibi | admin0: mysql> select * from migrate_version; | |
| 13:03:34 | gibi | | repository_id | repository_path | version | | |
| 13:03:39 | sean-k-mooney | admin0: nova-manage db version | |
| 13:03:40 | gibi | | nova | /opt/stack/nova/nova/db/sqlalchemy/migrate_repo | 422 | | |
| 13:03:43 | gibi | +---------------+-------------------------------------------------+---------+ | |
| 13:03:57 | gibi | admin0: or wha sean-k-mooney suggessts | |
| 13:04:14 | sean-k-mooney | ^ is teh db table its reading form | |
| 13:04:40 | sean-k-mooney | so if you have a backup you have not loaded gibi's way might be quicker to grep for | |
| 13:05:04 | sean-k-mooney | there is also nova-manage api_db version | |
| 13:05:06 | bauzas | gibi: as I said in my last comment, the current API is not RESTful either way | |
| 13:05:18 | sean-k-mooney | nova-manage db version tells you the cell db version | |
| 13:06:23 | sean-k-mooney | bauzas: the resful way to update the parent uuid woudl be jsut to update it | |
| 13:06:35 | sean-k-mooney | so a put to resource_providers/{uuid} | |
| 13:06:41 | sean-k-mooney | with the uuid updated in the body | |
| 13:07:04 | bauzas | sean-k-mooney: technically, put would be for updating the whole resource | |
| 13:07:22 | sean-k-mooney | if you wanted to do a partial update we woudl use patch | |
| 13:07:24 | bauzas | but we're nitpicking | |
| 13:07:29 | sean-k-mooney | but we dont use patch in openstack | |
| 13:07:44 | bauzas | my biggest concern comes to the fact I feel a qs param is just hacky | |
| 13:07:49 | sean-k-mooney | it often comes up but we keep saying no to patch | |
| 13:08:06 | bauzas | just for the idea of "oh shit, we need to do *something*" | |
| 13:08:15 | sean-k-mooney | bauzas: well i was orginaly suggesting just allowing this by defult | |
| 13:08:49 | bauzas | sean-k-mooney: I proposed three alternatives : one be PATCH, one be adding a new HTTP header | |
| 13:09:01 | bauzas | and the last being to have a specific subresource | |
| 13:09:02 | sean-k-mooney | im not conviced we need to have a flag of any kind to opt into it but im not against having one | |
| 13:09:20 | bauzas | sean-k-mooney: see my comments on PS2, I was thinking like you | |
| 13:09:33 | bauzas | but looks like we agreed at the PTG to sanity check this | |
| 13:09:46 | sean-k-mooney | ya the header occured to me but not sure we have precendent for that | |
| 13:09:52 | sean-k-mooney | we did | |
| 13:10:33 | sean-k-mooney | i did not have stong feels for or against added a sanity check with a slight one for not doign that | |
| 13:10:46 | sean-k-mooney | ill go read your comment and catch up | |
| 13:11:32 | gibi | I'm on the side of either not add a check (but we agreed on the PTG to add it) or if we add then for me the extra parameters is better fits to the QS than to header or body or subresource | |
| 13:11:59 | bauzas | gibi: hence my +1 :) | |
| 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: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 | |