| opendevreview | Merged openstack/nova master: Remove the periodic Centos 8 job https://review.opendev.org/c/openstack/nova/+/858272 | 04:58 |
|---|---|---|
| opendevreview | ribaudr proposed openstack/nova master: Default Nova persistent objects without soft delete. https://review.opendev.org/c/openstack/nova/+/854355 | 08:13 |
| Uggla | stephenfin, if you have some time can you have a look at https://review.opendev.org/c/openstack/openstacksdk/+/853949 | 08:22 |
| Uggla | gibi, bauzas , Hi so if you can have a look at https://review.opendev.org/c/openstack/nova/+/854355. | 08:55 |
| bauzas | Uggla: I'll try, a bit busy today due to some late stuff | 08:56 |
| Uggla | gibi, if you are motivated, then you can continue the review of manila/virtiofs all patches are under this new topic bp/manila_shares_attachments_v2 | 08:57 |
| Uggla | bauzas, I know, there is no hurry. | 08:58 |
| opendevreview | Sahid Orentino Ferdjaoui proposed openstack/nova master: compute: enhance compute evacuate instance to support target state https://review.opendev.org/c/openstack/nova/+/858383 | 08:58 |
| opendevreview | Sahid Orentino Ferdjaoui proposed openstack/nova master: api: extend evacuate instance to support target state https://review.opendev.org/c/openstack/nova/+/858384 | 08:58 |
| bauzas | Uggla: so, I'll look at your patch, but I'll also like to have dansmith to dent it https://review.opendev.org/c/openstack/nova/+/854355 | 09:37 |
| gibi | Uggla: yes. I would like to keep reviewing the manila work. It is a bit hard for me to balance my time at the moment. So I would like to ask you to be peristent asking for review :) I won't be offended if you keep pinging me all the time with the manial stuff :) | 09:46 |
| gibi | dansmith: if you have time when you are up we need your ovo experties here https://review.opendev.org/c/openstack/nova/+/854355/3..4#message-90730ecb520b41d974a8d64c7f7cd0bb0f56356c | 10:05 |
| gibi | Uggla: ^^ I see the problem but I don't see the imediate solution | 10:06 |
| gibi | and I don't like duplicating created_at | 10:06 |
| gibi | It might turn out that what I want is not possible with ovo | 10:07 |
| sean-k-mooney | i feel like what you want should be possible | 10:12 |
| sean-k-mooney | but i have not looked at the review properly | 10:12 |
| sean-k-mooney | i might pull it locally quickly and play with it for a bit and see if we can drop those fields | 10:13 |
| opendevreview | Stephen Finucane proposed openstack/nova master: conf: Add '[database] enable_soft_delete' flag https://review.opendev.org/c/openstack/nova/+/860401 | 11:33 |
| stephenfin | I've been thinking about that for years 👆 Maybe it belongs in oslo.db but the idea is what's relevant. Would welcome input (particularly melwitt) | 11:34 |
| sean-k-mooney | stephenfin: you were not in some of our interal team calls but we were discussign coudl we move to remvoing shaddow tables | 11:51 |
| sean-k-mooney | soft delete i dont have a probalem with honestly | 11:51 |
| sean-k-mooney | i dont like shadown tables | 11:52 |
| sean-k-mooney | my only concern with the conf approch is it change api behavior | 11:52 |
| sean-k-mooney | specificly for soft deleteing instance as we can use the restore api action to undelete them | 11:52 |
| sean-k-mooney | there is no api impact to remvoe shadow tables however since once we have archive the rows there is no going back | 11:53 |
| sean-k-mooney | so that seam less in vasive to me | 11:53 |
| sean-k-mooney | stephenfin: but yes removing softdelete is someting to consider. we could for example use the soft delete timout as the config option instead | 11:54 |
| sean-k-mooney | so if its not enable dthen just delete thing fully | 11:54 |
| sean-k-mooney | if we had already disabeld shadow tabels or remvoe them i think we could do that | 11:55 |
| stephenfin | sean-k-mooney: Aren't those different things? | 11:55 |
| sean-k-mooney | there are 3 things | 11:55 |
| stephenfin | soft-deleting an instance vs soft-deleteable tables | 11:55 |
| sean-k-mooney | soft deleteing an instnace marks the row as deelte but does not actully delete it until the soft delete timeout expires | 11:56 |
| stephenfin | I don't think it does | 11:56 |
| *** blarnath is now known as d34dh0r53 | 11:56 | |
| sean-k-mooney | i woudl have to check but as far as im aware it at least updates teh state so that it nolonger shows up in insntace list | 11:56 |
| sean-k-mooney | unless you add --deleted | 11:56 |
| sean-k-mooney | if it truely is independed and not marking it as delete in the db | 11:57 |
| stephenfin | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3363-L3377 | 11:57 |
| sean-k-mooney | then yes the 3 things (shadown tabels, soft deletable instance and soft deletable rows) would be independent | 11:57 |
| stephenfin | it seems to be setting power_state, vm_state and task_state fields. It's not setting 'deleted' or 'deleted_at' | 11:58 |
| sean-k-mooney | ok so its via a vm state | 11:58 |
| stephenfin | yup | 11:58 |
| stephenfin | which this doesn't touch | 11:58 |
| sean-k-mooney | ok then ya no api impact so | 11:58 |
| *** tbachman_ is now known as tbachman | 11:58 | |
| sean-k-mooney | add it as a ptg topic | 11:58 |
| sean-k-mooney | so long term i like to remvoe nova-manage acive_delete_rows and make pruge act like purge in other project | 11:59 |
| sean-k-mooney | in other projects purge it deletes the soft deleted rows | 12:00 |
| sean-k-mooney | but if we can also remvoe those (optionally or eventually permently) i woudl be happy to do that too | 12:00 |
| sean-k-mooney | stephenfin: i think i have mentioned this to you before but i woudl prefer if we mvoed to recommendign peole use https://github.com/ovh/osarchiver/ | 12:01 |
| sean-k-mooney | so if they have audit usecases they woudl enable soft-delete but use osarchiver to move them to there archival storage and out of our db | 12:02 |
| sean-k-mooney | if you can do that we dont need shadow tabels. if you dont have that usecase then you can disabel the soft-delete feature | 12:03 |
| sean-k-mooney | and upstream in B or C we could defautl to disabling it. | 12:04 |
| stephenfin | added to the agenda | 12:05 |
| sean-k-mooney | cool im goign to summeries my toughts on the gerrit commit | 12:08 |
| sean-k-mooney | ok done https://review.opendev.org/c/openstack/nova/+/860401/1#message-6ff04f04d84a33ff65f08e6c3956ea194a2a915c hopefully that makes sense | 12:16 |
| stephenfin | lgtm | 12:24 |
| sean-k-mooney | i like the idea of doing this in oslo too by the way but ya either could work | 12:43 |
| *** kopecmartin is now known as kopecmartin|sick | 12:56 | |
| *** dasm|off is now known as dasm | 13:43 | |
| stephenfin | Uggla: I figured out what was wrong with https://review.opendev.org/c/openstack/nova/+/854355/ Do you mind if I push my changes? | 15:32 |
| stephenfin | I'm leaving a comment now too | 15:32 |
| Uggla | stephenfin, cool, no pb. | 15:33 |
| Uggla | stephenfin, btw did you have time to the openstacksdk patch ? | 15:33 |
| stephenfin | Yeah, I think I +2d it | 15:34 |
| Uggla | \o/ | 15:34 |
| opendevreview | Stephen Finucane proposed openstack/nova master: objects: Add NovaSoftDeleteObject mixin https://review.opendev.org/c/openstack/nova/+/854355 | 15:40 |
| stephenfin | Uggla: lmk what you think ^ | 15:40 |
| Uggla | stephenfin, i'll have a look after our meeting. | 15:46 |
| opendevreview | Stephen Finucane proposed openstack/nova master: rpc: Mark attributes as private https://review.opendev.org/c/openstack/nova/+/792803 | 15:47 |
| melwitt | kashyap: the "abort live migration if monitoring fails" patch was to fail in a proper way when the error is encountered, there is another patch that needs review that will do the actual ignoring of the particular error https://review.opendev.org/c/openstack/nova/+/852002 | 15:55 |
| melwitt | kashyap: there was an issue with the patch I had proposed to workaround it, so I abandoned it. ^ is the new one from another contributor | 15:59 |
| Uggla | stephenfin, what you did in https://review.opendev.org/c/openstack/nova/+/854355/4..5, sounds good to me. Thank you. Now let's check if gibi, dansmith, bauzas agree. | 16:02 |
| * bauzas looks when we deprecated | 16:02 | |
| bauzas | I thought we said we haven't wanted to have API tables to be soft-deletable | 16:03 |
| bauzas | but, we haven't said "yeah, we should deprecate the other tables" | 16:03 |
| dansmith | yeah | 16:05 |
| dansmith | I don't agree with the use of "deprecated" here | 16:05 |
| dansmith | "not recommended for everything by default" makes sense | 16:05 |
| bauzas | at least I'm afraid of saying "we deprecate instance record soft-deletion" | 16:07 |
| dansmith | was there some decision to deprecate and actually remove this stuff? because I think I disagree with that | 16:07 |
| *** labedz is now known as labedz_ | 16:07 | |
| dansmith | and if not, we should change the wording in the patch I think | 16:07 |
| bauzas | Uggla: I looked at your patch | 16:11 |
| kashyap | melwitt: Thanks for jogging my memory! I now recall | 16:11 |
| bauzas | sounds quite good to me if you say 'we need to formally name which tables do softdelete" | 16:12 |
| bauzas | which is what you code | 16:12 |
| kashyap | melwitt: I thought this one from Brett has already merged...but apparently not yet. Is it waiting on something still? | 16:12 |
| melwitt | kashyap: just needs a second reviewer | 16:12 |
| kashyap | Ah, nod. I thought something else besides it. | 16:13 |
| melwitt | I already +2ed it | 16:13 |
| melwitt | nah | 16:13 |
| kashyap | gibi: or any other core who's not Mel, can you please put this through? - https://review.opendev.org/c/openstack/nova/+/852002 | 16:14 |
| kashyap | melwitt: Also thank you for - https://review.opendev.org/c/openstack/nova/+/859358/1 | 16:14 |
| melwitt | :) | 16:15 |
| kashyap | Often these unit tests take a ton of time (at least for me), and I keep duking around them | 16:15 |
| gibi | kashyap: I added to my queue but no promises when I get to it | 16:15 |
| kashyap | gibi: What? I thought you'd attach a promiese-to-be-executed-on-this-date to all your reviews! | 16:16 |
| melwitt | kashyap: they take a ton of time for me, pretty much never goes smoothly 😆 | 16:17 |
| kashyap | melwitt: Good to know; I feel particularly low when a unit test that I'm struggling with takes so long that a hen will develop teeth, but the test won't come out right. | 16:18 |
| melwitt | kashyap: "hen develop teeth" haha I've never heard that before | 16:19 |
| kashyap | I recently learnt the expression, "as rare as hen's teeth" -- https://en.wiktionary.org/wiki/rare_as_hen%27s_teeth :P | 16:21 |
| melwitt | it's funny :) | 16:22 |
| kashyap | On that note, /me goes to make "non-hen" dinner | 16:24 |
| melwitt | o/ | 16:25 |
| gibi | kashyap: :) | 16:33 |
| opendevreview | Stephen Finucane proposed openstack/nova master: objects: Remove unnecessary type aliases, exceptions https://review.opendev.org/c/openstack/nova/+/738240 | 16:36 |
| opendevreview | Stephen Finucane proposed openstack/nova master: objects: Use imports instead of type aliases https://review.opendev.org/c/openstack/nova/+/738018 | 16:36 |
| opendevreview | Stephen Finucane proposed openstack/nova master: objects: Remove wrappers around ovo mixins https://review.opendev.org/c/openstack/nova/+/738019 | 16:36 |
| opendevreview | Stephen Finucane proposed openstack/nova master: objects: Remove 'NovaObjectDictCompat' from 'Service' https://review.opendev.org/c/openstack/nova/+/835595 | 16:36 |
| opendevreview | Stephen Finucane proposed openstack/nova master: WIP: add ovo-mypy-plugin to type hinting o.vos https://review.opendev.org/c/openstack/nova/+/758851 | 16:36 |
| opendevreview | Stephen Finucane proposed openstack/nova master: objects: Add NovaSoftDeleteObject mixin https://review.opendev.org/c/openstack/nova/+/854355 | 17:29 |
| sean-k-mooney | stephenfin: to be clear the goal of ^ shoudl be the minimal possible change to allow Uggla to create teh manilla share object models without delete/deleted at in the db or ovo | 17:30 |
| stephenfin | Yup, I think that's doing that? I suspect dansmith and bauzas had missed part of the commit message that explained that | 17:33 |
| stephenfin | "Currently, the NovaPersistentObject mixin includes fields required by the soft delete feature - deleted and deleted_at - even if the backing SQLAlchemy model isn't using soft delete." | 17:34 |
| stephenfin | 👆 that bit | 17:34 |
| sean-k-mooney | right but other way to do that is for the manilla object to inherit form the timestamed one | 17:35 |
| sean-k-mooney | and make no change to anything else | 17:35 |
| sean-k-mooney | so instead of inheriting form NovaPersistentObject the manila share ones could inherit form ovoo_base.TimestampedObject which is also called NovaTimestampObject | 17:36 |
| sean-k-mooney | we dont need to add the mixin for the orgianl usecse | 17:36 |
| stephenfin | Same thing, different approach. If I was Uggla, I'd probably do that to avoid this blocking things | 17:36 |
| sean-k-mooney | gibi: Uggla did i miss why we are not just doing that | 17:36 |
| sean-k-mooney | stephenfin: thats basically what i suggested a month ago https://review.opendev.org/c/openstack/nova/+/854355/5#message-839b7637eabf1d6dcf4e95050e9244c954dc1056 | 17:37 |
| sean-k-mooney | at that time i did not see that we had NovaTimestampObject | 17:37 |
| sean-k-mooney | and did not need ot intoduce a new class at all | 17:38 |
| stephenfin | That change does still make sense though. As unlikely as it is that anyone will bump those major versions, it is confusing and the TODOs are helpful to highlight that | 17:38 |
| sean-k-mooney | i dont dissagree that we might also want to do this | 17:39 |
| sean-k-mooney | but i dont think we want to do this in the manilla share seriese | 17:39 |
| stephenfin | agreed | 17:39 |
| sean-k-mooney | which si why i ask Uggla to not do this when i first reviewed | 17:39 |
| gibi | I cannot recall the reason we went that way | 17:40 |
| gibi | probably to fix the other ovos where we can fix | 17:40 |
| sean-k-mooney | gibi: Uggla orgingial patch alredy had updated all the other objects | 17:41 |
| Uggla | sean-k-mooney, stephenfin I tried to make objects without soft delete the default behavior to avoid future misused. I also started with a less intrusive patch, but gibi wanted to extend it. | 17:41 |
| sean-k-mooney | Uggla: ya but if we did that it shoudl be a sperate blueprint/spec not mixed into your share work | 17:41 |
| sean-k-mooney | bauzas: melwitt when i mentioned stpehens change in the bug call by the way i ment https://review.opendev.org/c/openstack/nova/+/860401 | 17:43 |
| sean-k-mooney | not https://review.opendev.org/c/openstack/nova/+/854355 | 17:43 |
| sean-k-mooney | i just saw that stephen updated that also | 17:44 |
| melwitt | sean-k-mooney: thanks | 17:44 |
| Uggla | sean-k-mooney, ok if it is needed. | 17:51 |
| *** adia is now known as abdi | 18:09 | |
| *** abdi is now known as adia | 18:15 | |
| *** adia is now known as abdi | 18:53 | |
| *** dasm is now known as dasm|off | 21:34 | |
| atmark | I messed up nova cells and now computes states are down. I corrected the cells but they still show as down. How can I reregister the computes? | 22:17 |
| atmark | This is a new deployment | 22:17 |
| atmark | `openstack compute service delete id` won't let me delete | 22:18 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!