| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-20 | |||
| 18:52:49 | mriedem | leakypipes: melwitt: still need a final +W on that gate fix for the anti-affinity multi-create stuff (and the test change below it) https://review.openstack.org/#/c/583347/ | |
| 18:52:53 | mriedem | as i said in the meeting, it's cleaner now | |
| 18:52:57 | mriedem | b/c i removed the alternates stuff | |
| 18:53:57 | mriedem | fried_rice: you should probably just +W this https://review.openstack.org/#/c/576693/ - i know you cleaned it up a bit but it's so trivial it's not really worth sitting on it | |
| 18:54:57 | leakypipes | mriedem: k, reviewing now. | |
| 18:55:02 | fried_rice | mriedem: I effectively changed the whole logic of it, which is why I hadn't felt great about pushing it myself. | |
| 18:55:54 | mriedem | yeah but it was like a 1 line conditional... | |
| 18:55:59 | mriedem | up to you | |
| 18:56:03 | mriedem | but i think it's ok in this case | |
| 18:56:04 | mriedem | personally | |
| 18:57:38 | fried_rice | Oh, and btw, superdan, `is_bfv = foo and bar or False` <== curse ovo | |
| 18:58:02 | superdan | um, what? | |
| 18:58:28 | fried_rice | The fact that you would ever have to say `x and y or False` rather than just `x and y` is stupid. | |
| 18:59:28 | fried_rice | mriedem: Is retrying 3rd party?? | |
| 18:59:51 | fried_rice | I guess it must be, if it's in our requirements files... | |
| 18:59:56 | fried_rice | mahbad | |
| 19:02:21 | mriedem | superdan: for context on random things on your first day back https://review.openstack.org/#/c/580720/4/nova/compute/api.py@892 | |
| 19:02:47 | mriedem | fried_rice: that wasn't an ovo thing | |
| 19:02:49 | superdan | fried_rice: that's a python thing. what does it have to do with ovo? | |
| 19:02:52 | superdan | yeah, that | |
| 19:02:55 | mriedem | it was because the bdm list doesn't need to have a root bdm | |
| 19:03:10 | mriedem | you can create a server with volumes attached to it which aren't the root disk | |
| 19:03:51 | fried_rice | No, it was because you can assign a False-ish non-boolean to a BooleanField, but if that False-ish is None, it freaks out when it tries to "coerce" it. | |
| 19:05:50 | fried_rice | I would buy it if you could only assign True rather than any other True-ish (except I still would have expected the BooleanField to do the casting). But it doesn't care if you assign other True-ish values, like whatever's in `root`. | |
| 19:06:28 | superdan | fried_rice: that's because it's not nullable and ovo specifically checks for None-ness | |
| 19:06:45 | superdan | given how often boolean coercions confuse things, I don' | |
| 19:06:50 | superdan | don't think I'd "curse ovo" for that :) | |
| 19:07:21 | mriedem | i would curse your bones availability zones | |
| 19:09:28 | fried_rice | OVOClass(non_nullable_bool_field=None) can be distinguished from OVOClass() ('non_nullable_bool_field' in kwargs, kind of thing) which might make more sense from a consumer standpoint. | |
| 19:10:18 | fried_rice | I suppose ovoinstance = None is the established mnemonic for "null this field". | |
| 19:10:35 | fried_rice | Is it possible for a BooleanField to be nullable? | |
| 19:10:38 | fried_rice | The whole thing is gross. | |
| 19:12:30 | tssurya | superdan, mriedem: sorry having a bad internet connection, but.. by default queued_for_delete would be False right ? not NULL.. | |
| 19:12:42 | superdan | tssurya: on the object, but not in the database | |
| 19:13:06 | superdan | tssurya: your default=False in the model schema doesn't mean it's actually set that way in SQL, only in SQLA's view | |
| 19:15:32 | tssurya | I thought I remembered my devstack DB to have False by default after the migration.. | |
| 19:46:28 | openstackgerrit | Matt Riedemann proposed openstack/nova master: compute: Ensure pre-migrating instances are destroyed during init_host https://review.openstack.org/562284 | |
| 19:54:02 | openstackgerrit | Dan Smith proposed openstack/nova master: Online data migration for queued_for_delete flag https://review.openstack.org/584504 | |
| 19:54:04 | superdan | mriedem: tssurya ^ | |
| 19:54:29 | mriedem | that can't be correctly done, it was too fast | |
| 19:54:42 | superdan | definitely not | |
| 19:55:05 | superdan | I think I need at least one more test for an edge case there, | |
| 19:55:18 | superdan | but I was shooting to replicate the functional test in the existing patch | |
| 19:56:00 | superdan | and going fast because I feel bad for throwing a wrench into things at this point | |
| 19:56:15 | superdan | so hopefully that's enough to decide if we want to pursue or not | |
| 19:59:13 | mriedem | comments inline | |
| 19:59:22 | mriedem | taint bad | |
| 20:01:25 | superdan | we'll see what tssurya thinks | |
| 20:02:35 | mriedem | also need to test the limit | |
| 20:02:39 | mriedem | but those are my 2 big points | |
| 20:02:52 | superdan | yeah the limit was the missing gap I was thinking of | |
| 20:03:17 | mriedem | obligatory https://www.youtube.com/watch?v=vT8OU5WtfkQ | |
| 20:03:35 | mriedem | i want to be tony, you can be that other guy | |
| 20:04:03 | tssurya | well, considering you are having to change it to NULL from False, as per the commit message means the values were False by default right ? | |
| 20:04:28 | superdan | tssurya: no | |
| 20:04:39 | superdan | tssurya: they're NULL in the db, but SQLA hides it from us | |
| 20:05:05 | tssurya | ah, okay yes this is what you were saying before I got disconnected | |
| 20:05:48 | tssurya | sorry, as for the technique I don't have any issues in making this multi-cell aware | |
| 20:14:42 | superdan | mriedem: hmm, stumped on the soft_deleted=True part | |
| 20:14:51 | superdan | I'm not gettin those instances back from my filter query | |
| 20:15:29 | superdan | but I think I do if I don't provide a uuid filter... | |
| 20:17:29 | mriedem | superdan: see the test in the other patch? | |
| 20:17:39 | mriedem | you need to set the vm_state on a couple of the servers | |
| 20:17:46 | mriedem | non-deleted services | |
| 20:17:54 | mriedem | *servers | |
| 20:18:00 | superdan | right, | |
| 20:18:01 | superdan | I'm saying I do that and I don't get them back from my query | |
| 20:18:28 | melwitt | are you talking about this logic? https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2105 | |
| 20:19:32 | superdan | generally, yes, but I'm saying I don't get the instances back that I expect if I include a uuid= filter as well | |
| 20:20:22 | melwitt | if you use 'deleted': False you won't get soft_deleted instances | |
| 20:20:40 | superdan | yes, this is deleted=True, soft_deleted=True | |
| 20:20:51 | melwitt | okay | |
| 20:20:59 | mriedem | should be an OR | |
| 20:21:05 | mriedem | https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2112 | |
| 20:21:12 | superdan | yup | |
| 20:21:36 | mriedem | oh the uuid filter, hmm | |
| 20:21:45 | mriedem | you don't get the soft deleted instances or any instances? | |
| 20:22:04 | superdan | I get some but not all, let me see which ones specifically | |
| 20:22:47 | superdan | yeah I get the non-soft-deleted ones | |
| 20:23:25 | mriedem | should handle your UUID list here https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2426 | |
| 20:23:34 | superdan | yeah | |
| 20:24:01 | superdan | okay I actually don't think the uuid filter is breaking me, I'm just getting both deleted instances in that case, | |
| 20:24:07 | superdan | but not the soft_deleted ones ever | |
| 20:27:28 | superdan | hmm, my .save wasn't ... saving | |
| 20:28:35 | melwitt | okay, at least there's a reason that makes sense. just kept looking at this logic thinking, "this should work" | |
| 20:28:56 | superdan | well, I dunno that it makes sense, but at least it means it wasn't the query logic :) | |
| 20:29:24 | melwitt | correction: it makes sense that if your save is failing, you don't find the soft_deleted vm_state you saved :P | |
| 20:30:11 | superdan | true story | |
| 20:32:28 | mriedem | "it hurts when i do this" "don't do that" | |
| 20:32:59 | superdan | ^ the core of much of my debugging | |
| 20:33:28 | openstackgerrit | Pierre Blanc proposed openstack/nova master: Docs: Add guide to migrate instance with snapshot https://review.openstack.org/584442 | |
| 20:34:04 | openstackgerrit | Dan Smith proposed openstack/nova master: Online data migration for queued_for_delete flag https://review.openstack.org/584504 | |
| 20:48:04 | mriedem | fried_rice: i think we can get the cpu traits thing in early next week, mostly small things to address from me | |
| 20:48:18 | fried_rice | mriedem: okay. | |
| 20:48:22 | mriedem | he did add something which gets logged at debug 450 times though... | |
| 20:48:30 | mriedem | which seems excessive if it's not something we need to care about | |
| 22:05:45 | openstackgerrit | Eric Fried proposed openstack/nova master: docs: add nova host-evacuate command to evacuate documentation https://review.openstack.org/578040 | |
| #openstack-nova - 2018-07-21 | |||
| 00:12:14 | openstackgerrit | melanie witt proposed openstack/nova master: DNM: Don't overwrite greenthread-local context in target_cell https://review.openstack.org/582332 | |
| 00:45:34 | openstackgerrit | melanie witt proposed openstack/nova-specs master: Add a script for counting blueprints https://review.openstack.org/581914 | |
| 01:49:01 | openstackgerrit | Merged openstack/python-novaclient master: Add support for microversion 2.65 https://review.openstack.org/583632 | |