| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-05-25 | |||
| 11:02:33 | sean-k-mooney | lyarwood: are you currently wroking on a new reviewion? | |
| 11:02:42 | sean-k-mooney | *revision | |
| 11:03:21 | lyarwood | sean-k-mooney: I was trying to but tbh I'm totally distracted waiting for a phone call from the hospital, I'll wait for your review and respin it later today if that's okay | |
| 11:03:58 | lyarwood | /me has been waiting for the call since 8:30am, damn NHS. | |
| 11:08:17 | sean-k-mooney | lyarwood: ok i dont know if i have any real changes that i want you to make there are thing we discussed previosly but i expect any comment i add to be relitvly minor | |
| 11:08:37 | lyarwood | ack kk | |
| 12:05:58 | masterpe | Hi, In train when I use the command ./nova-manage db archive_deleted_rows with --before it will a long time before it completed the command. Even tho I use a --max_rows 1000. Without the --before option it is a lot faster. When I do a SHOW INDEX FROM nova.instances; on the database I think I don't see any indexes on the deleted_at column. | |
| 12:08:57 | sean-k-mooney | masterpe: --before is intedned to be run requently as in daily | |
| 12:09:17 | sean-k-mooney | so typicaly it would be used to delete rows over say 90 days | |
| 12:09:35 | sean-k-mooney | and you would do that daily so its only removeing 1 days worth at a time | |
| 12:10:38 | masterpe | Without before it is doing everything? | |
| 12:11:24 | sean-k-mooney | if you dont set anything i think so | |
| 12:12:08 | sean-k-mooney | its slighly painful to do the initall catch up but once you get it to the time period you want to retain --before is generally our recomended way to do this | |
| 12:35:17 | bauzas | gibi: sorry, I was afk | |
| 12:35:48 | sean-k-mooney | bauzas: im making my way through your mdev spec currently too by the way | |
| 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 | |