Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-01-09
22:02:10 dansmith I guess we could do the init defaulting part yeah
22:02:18 efried Could do it in nova as a stopgap until we can suck in the ovo release
22:02:41 dansmith yup
22:02:48 efried and I can see making an infrastructure that poisons use of obj_set_defaults in __init__ unless inheriting from Ephemeral
22:03:05 efried ...to enforce the paradigm
22:03:07 dansmith I thought it was going to involve a little more mucking with obj_set_defaults() than it did, but I was also planning both of these together
22:03:35 dansmith efried: we can test for it in test_objects I think
22:03:44 dansmith efried: instantiate every object (like we do) and make sure that unless it inherits from that mixin, nothing is set after init
22:04:41 efried well, that doesn't prove obj_set_defaults wasn't invoked. I was thinking about making obj_set_defaults set a flag or something, then we can check for that flag in the test
22:05:05 dansmith well, we have (or had)objects in the past that didn't use set_defaults, but just set them explicitly in __init_
22:05:20 efried Is that also a bad thing?
22:05:21 dansmith just "self.foo = None" or something in __init__
22:05:32 efried I would think that would be okay.
22:05:33 dansmith it's the exact same thing, except less obvious, so.. yes?
22:05:42 dansmith it's the *exact* same outcome :)
22:05:42 efried it's just the blanket obj_set_defaults we want to avoid
22:06:08 dansmith you make it sound like it's just an arbitrary rule... the reason for the rule is because of the clobbering of columns
22:07:36 efried which is never ever desirable?
22:08:07 dansmith for a row-based object, never ever
22:08:28 dansmith and without a system in place, people see and copy examples, which is why we have a few in the tree
22:08:49 dansmith any time I or mriedem would catch it in review, we'd get it taken out, but things slip through,
22:09:01 dansmith then people see that example, figure it's okay, etc
22:09:21 efried yeah, I get it. So, an enforcing test.
22:09:35 dansmith so what I'm trying to say is let's make it a little more obvious, you can only do this if you inherit from ephemeral, but you better not be row-based
22:10:02 dansmith all our objects used to be basically 1:1 mappings for the db, but over time we've grown a lot more of these general purpose ones, so it used to be easy to say "you can never ever do this"
22:10:30 efried really, we could do the test without introducing any new semantics. Course, that would mean actually fixing the existing violations, immediately. Which we can put off if we introduce the new semantics.
22:10:39 efried so, I dig.
22:11:12 dansmith you mean we can do the test now and fix the instances we have, right?
22:11:32 dansmith that's true, but I think the point here is, it's legit to want sane defaulting for certain types of objects so let's open the door but in a controlled way
22:11:53 dansmith so anything in tree that does this now either should be inheriting from the new base, or their __init__ behavior needs be to cleaned up
22:12:40 efried If we set up the Ephemeral infrastructure, we can just blindly make those objects inherit from it so we're no worse off than we were before, and fix them "later", but new work will have to be compliant
22:12:40 efried but
22:12:40 efried If we set up the test now, we have to fix the existing objects that violate the rule
22:12:40 efried I mean:
22:13:11 dansmith well, I don't want to blindly do it, I want to examine each case, but there aren't that many that I know of (other than notifications I think)
22:13:19 dansmith but otherwise yes
22:13:29 efried okay, I'm on board.
22:13:31 dansmith no reason to fix them only to open the door to the thing a patch later
22:14:39 dansmith So tomorrow I will copy that into nova, work on the test and see where we are
22:15:44 efried just since it's in my head, would it make sense for the long-term solution to instead use a class variable on VersionedObject, EPHEMERAL=False, which ephemeral subclasses could override to True?
22:15:47 efried instead of a mixin
22:16:33 dansmith doing that means you could have an ephemeral parent and be non-ephemeral yourself, which would be bad
22:16:38 dansmith or the opposite
22:17:24 dansmith you saw my reply that nova things will not need to mixin themselves right?
22:17:37 dansmith there will be a base class in base.py which mixes in and then you'll inherit from that once
22:18:39 dansmith and that means we can always isinstance(thing, NovaEphemeral) to know its true nature
22:19:38 efried as opposed to self.EPHEMERAL. But yeah, the possibility of flipping back and forth in nested subclasses is a reasonable argument against that.
22:20:31 dansmith yeah I just mean you can't really fake your parentage easily
22:20:37 efried though this way you can still do that, but you can only flip *to* ephemeral, and you can only do it once.
22:21:33 efried unless we just don't define the mixin at all. Make NovaEphemeral a subclass of NovaObject.
22:21:36 efried anyway, details.
22:22:05 dansmith yeah, just being a class variable makes it changeable at runtime, vs an essential quality of the thing
22:22:31 dansmith what's the argument for it being a class variable?
22:22:50 dansmith less formality, maybe, but I'm going for formality here
22:22:59 efried "efried doesn't like mixins"?
22:23:36 dansmith oh I don't really either, but I think I was asked to make those qualities mixins because that's how python libraries do things like this without affecting interfaces as much
22:23:48 efried makes it tough to figure out what's happening
22:23:59 efried ye gods, have you seen the openstacksdk project?
22:24:13 dansmith yep, totally agree, which is why the one class that does the mixin in base and then you single-inherit from that, knowing its done right
22:24:20 dansmith mixin heaven?
22:24:32 efried every class has like a dozen mixins, and it's freaking impossible to understand where you go when you invoke a method
22:24:43 dansmith yeah, I'm totally with you on that
22:24:51 dansmith when this was all code in nova there was a lot less mixing going on
22:24:53 efried mixins, and proxy shims, and dynamic construction, and ...
22:25:01 dansmith but it's a library now, so..
#openstack-nova - 2020-01-10
01:16:08 openstackgerrit Merged openstack/nova master: Add missing parameter vdi_uuid in log message https://review.opendev.org/701004
01:29:59 alex_xu efried: sean-k-mooney sorry, I was too sleepy last night, I didn't follow the end of dicussion for placement-ese and flavor-ese, I saw there are some vision in the irc log, but what should we do for mix instance spec in this release?
01:45:06 openstackgerrit zhufl proposed openstack/nova master: Fix typos in nova doc https://review.opendev.org/701876
02:44:46 openstackgerrit Merged openstack/nova master: Use Placement 1.35 (root_required) https://review.opendev.org/699050
05:29:28 LiangFang dansmith: happy new year:) Could you please help to review again: https://review.opendev.org/#/c/689070/
05:31:41 LiangFang related code implemetation are also ready except UT: https://review.opendev.org/#/c/663542/
08:41:45 openstackgerrit Merged openstack/nova master: Add a workaround config toggle to refuse ceph image upload https://review.opendev.org/657078
08:46:05 shilpasd Hi, thanks for review for DB patch for ignore root_gb
08:46:18 shilpasd brinzhang_: thanks for review for DB patch for ignore root_gb
08:46:33 shilpasd regarding comment related to release note, IMO since its code level changes, we shouldn't have
08:47:01 brinzhang_ shipasd: Yeah, I am ok.
08:47:27 shilpasd brinzhang_: thanks
08:48:05 brinzhang_ shilpasd: np :)
08:50:03 openstack Launchpad bug 1663456 in OpenStack Compute (nova) "Field 'updated_at' always 'None' when show aggregate" [Low,Confirmed]
08:50:03 brinzhang_ alex_xu: I am reviewing this bug https://bugs.launchpad.net/nova/+bug/1663456, I think Vishakha Agarwal fix is available, can you restore https://review.opendev.org/#/c/580271/
08:51:12 brinzhang_ alex_xu: it's abandoned by matt because of the merge conflict, I will resolve the merge conflict, and let the bug continue
09:51:07 gibi dansmith: thanks for starting the discussion on the conflicting ovo usage. I appreciate your effort. I was alway too lazy to start up such a discussion. I will comment your patches soon
10:03:44 ralonsoh gibi, sorry, I updated the commit message at the same you +1 the patch https://review.opendev.org/#/c/701797/
11:28:44 gibi ralonsoh: no worries
11:28:50 gibi ralonsoh: thanks for fixing the gatae
11:28:52 gibi gate
11:41:59 openstackgerrit Luyao Zhong proposed openstack/nova-specs master: support live migration with virtual persistent memory https://review.opendev.org/695863
12:04:12 brinzhang gibi: sean-k-mooney: could you please review these specs? https://review.opendev.org/#/c/699669/ and https://review.opendev.org/#/c/682302/
12:05:00 brinzhang gibi, sean-k-moonkey: thanks~
12:16:01 sean-k-mooney efried: do you know if sundar has been testing his cyborg patches on centos 7 still. i have tried 3 different way to get python 3.6 running properly on centos and they all end up failing to stack with python errors so im swaping to ubunut just want to check if you know if he has tested ubuntu 18.04
12:19:23 sean-k-mooney i assume the tempest job proably used ubunut so it should work but just said i would ask
12:25:51 gibi brinzhang: added to my review queue
12:26:21 brinzhang gibi: thanks :)
12:31:31 gibi dansmith, efried: I commented both ovo improvement patch. I totally +2 on the EphemeralObject one, and I'm OK with the default_fn too but I don't see where I will use that in Nova
12:33:10 openstack Launchpad bug 1663456 in OpenStack Compute (nova) "Field 'updated_at' always 'None' when show aggregate" [Low,Confirmed] - Assigned to Brin Zhang (zhangbailin)
12:33:10 brinzhang gibi: Today I was reviewed this bug https://bugs.launchpad.net/nova/+bug/1663456,and reviewed it's fixed patch, I think it's available, and it was abandoned by matt, could you please restore this patch?
12:33:58 brinzhang gibi: I will resolve that merge conflict, and continue work for it.
12:47:49 gibi brinzhang: restored the review https://review.opendev.org/#/c/580271/
12:48:52 lyarwood did someone track down a change in devstack-gate that's causing n-net to be dropped from stable/queens cells-v1 runs?
13:01:23 frickler lyarwood: that should be fixed by https://review.opendev.org/701404

Earlier   Later