| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-13 | |||
| 18:02:39 | sean-k-mooney | so i was about to push what i think is a fix | |
| 18:02:40 | mriedem | ok i'm not sure what was used then, because we | |
| 18:02:40 | mriedem | TypeError: object of type 'filter' has no len() | |
| 18:02:59 | sean-k-mooney | it might be something else | |
| 18:03:15 | sean-k-mooney | does this show up in unit tests | |
| 18:03:22 | mriedem | nope, b/c there is no test coverage | |
| 18:03:25 | mriedem | the method is mocked out | |
| 18:03:42 | mriedem | lots of this old virt/disk/mount code isn't tested very deep | |
| 18:03:59 | sean-k-mooney | ok i guess i should add a test. i was hoping ot just quickly fix this and move on | |
| 18:04:08 | mriedem | it uses a stub, | |
| 18:04:09 | mriedem | _fake_detect_nbd_devices_none | |
| 18:04:18 | mriedem | which returns a [] rather than the results of a filter | |
| 18:04:55 | sean-k-mooney | right it shoudl be mocking the os.listdir call really | |
| 18:05:16 | sean-k-mooney | well the stub is fine for oter tests | |
| 18:05:34 | sean-k-mooney | but we should have a test for that _detect_nbd_devices function directly. | |
| 18:09:29 | mriedem | yeah i see there is a _fake_listdir_nbd_devices stub but it's not used by anything, i.e. removing that doesn't make tests fail | |
| 18:21:36 | mriedem | yeah so filter() in py27 returns a list, or some kind of iterable, and in py3 it's a built-in function | |
| 18:21:42 | mriedem | which is why random.shuffle doesn't handle it | |
| 18:27:40 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Introduces SDK to IronicDriver and uses for node.get https://review.opendev.org/642899 | |
| 18:27:41 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for node.list https://review.opendev.org/656027 | |
| 18:27:42 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for validating instance and node https://review.opendev.org/656028 | |
| 18:27:42 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for setting instance id https://review.opendev.org/659690 | |
| 18:27:42 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for add/remove instance info from node https://review.opendev.org/659691 | |
| 18:27:43 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for getting network metadata from node https://review.opendev.org/670213 | |
| 18:40:27 | openstackgerrit | sean mooney proposed openstack/nova master: lxc: make use of filter python3 compatiable https://review.opendev.org/676263 | |
| 18:43:21 | sean-k-mooney | mriedem: i think that should fix it ^ | |
| 18:43:42 | mriedem | yup just commented | |
| 18:43:48 | mriedem | i was working on the same thing | |
| 18:45:10 | sean-k-mooney | :) i can make the chagne you suggest but if you have tehm locally i can also abandon min | |
| 18:45:23 | sean-k-mooney | it will only take 30 secons either way | |
| 18:45:34 | mriedem | i dropped mine | |
| 18:45:55 | sean-k-mooney | ok ill make it a static method so it does not need to be an instance method at all | |
| 18:46:54 | sean-k-mooney | we dont traselate logs anymore right | |
| 18:47:38 | sean-k-mooney | you know what ill jsut fix this and i wont rewrite teh fule file | |
| 18:48:22 | sean-k-mooney | its tempting but i have no idea why we save sutff to self.error in the oter functions and i dont want to break that | |
| 18:48:25 | openstackgerrit | Merged openstack/nova master: Add TargetDBSetupTask https://review.opendev.org/627892 | |
| 18:48:34 | openstackgerrit | Merged openstack/nova master: Prevent init_host test to interfere with other tests https://review.opendev.org/675553 | |
| 18:49:48 | openstackgerrit | Eric Fried proposed openstack/nova master: Get ironicclient retry conf from ksa https://review.opendev.org/676264 | |
| 18:52:08 | efried | dustinc: Having done all that work... now I'm not sure it's the right thing at all. | |
| 18:52:15 | efried | dansmith: Why are those other methods remotable? | |
| 18:53:29 | dansmith | efried: there are some legacy cases where aggregate methods got called from the compute, like when the xen driver was doing some sync | |
| 18:53:44 | dansmith | and potentially also just people adding them because they see the decorator elsewhere and don't understand | |
| 18:53:53 | efried | yes, that's certainly what I would have done | |
| 18:58:43 | openstackgerrit | sean mooney proposed openstack/nova master: lxc: make use of filter python3 compatiable https://review.opendev.org/676263 | |
| 18:59:57 | sean-k-mooney | mriedem: done im going to grab something to eat so ill be afk for a while | |
| 19:06:23 | efried | o/ cfriesen! how are things? | |
| 19:06:55 | cfriesen | efried: just got back from long road trip with family. now recovering from my vacation | |
| 19:06:59 | cfriesen | :) | |
| 19:07:07 | efried | ah yes, I know the feeling | |
| 19:18:53 | mriedem | easy fix https://review.opendev.org/#/c/676263/ | |
| 19:21:31 | dansmith | mriedem: sean-k-mooney won't list() fix it without the six thing? | |
| 19:23:20 | mnaser | mriedem: about https://review.opendev.org/#/c/675496/ would it make sense to make a patch updating the index? | |
| 19:23:28 | mnaser | (in master, so that one alone is backportable) | |
| 19:24:25 | mriedem | dansmith: yeah i think so | |
| 19:25:00 | dansmith | mriedem: so am I missing why we don't just do that? I mean, this works too but I don't think it's what we've done elsewhere | |
| 19:25:29 | mriedem | that's what i was doing in my local patch, but sean-k-mooney beat me to it and used six here so... | |
| 19:25:35 | mriedem | i'm fine with removing the six.moves | |
| 19:26:03 | mriedem | mnaser: you mean change the unique index from just uuid to uuid + deleted? | |
| 19:26:09 | mnaser | yeah | |
| 19:26:12 | dansmith | I know he'll be back.. should we wait to see if he had some reason or just go ahead and fix it? | |
| 19:26:15 | dansmith | not sure if it's a rush | |
| 19:26:38 | mriedem | mnaser: the schema change is a bit wonkier b/c the unique index is defined on the uuid column rather than an actual index | |
| 19:26:49 | mriedem | it's not a rush, can wait for sean | |
| 19:27:17 | mnaser | mriedem: i guess the only issue with that is if you put your entire fleet into maintenance mode and then bring it back | |
| 19:27:20 | mnaser | you just doubled your db | |
| 19:27:32 | mriedem | mnaser: nvm, i guess it's not defined on the column https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L115 | |
| 19:27:35 | mnaser | so i think i rather not end up with that type of thing | |
| 19:27:37 | mriedem | it's just a unique index | |
| 19:28:38 | mnaser | my suggestion was to drop the update-deleted workaround | |
| 19:28:52 | mnaser | and make it so that it creates a new entry anyways | |
| 19:28:57 | mnaser | actually i havent had enough coffee | |
| 19:29:00 | mnaser | ignore me :) | |
| 19:31:31 | mriedem | what tz are you in? | |
| 19:32:03 | dansmith | he's a millenial | |
| 19:32:15 | mriedem | the order of the columns in the index also matters and i'm not sure what order we'd use for that unique index | |
| 19:32:16 | dansmith | so, start work at 11am, lunch at 12, etc | |
| 19:33:58 | efried | dustinc: You handy? | |
| 19:35:18 | efried | dustinc: I just put up https://review.opendev.org/676268 which, until it's merged, you should Depends-On from https://review.opendev.org/#/c/642899/ which should also bump the ksa minimum to 3.16.0. | |
| 19:35:35 | efried | mriedem: Is devstack automatically a required-projects? | |
| 19:36:13 | efried | i.e. can I Depends-On a devstack patch from nova without twaddling .zuul.yaml? | |
| 19:37:38 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-lxc job to the experimental queue https://review.opendev.org/676024 | |
| 19:38:06 | mriedem | you said twaddle | |
| 19:38:37 | mriedem | eventually the job inherits from devstack-base which requires devstack | |
| 19:40:59 | efried | okay, cool | |
| 20:31:22 | mriedem | efried: i volunteer alex_xu and gibi for the project update in shanghai | |
| 20:31:46 | efried | ++ | |
| 20:31:52 | efried | dual language | |
| 20:35:15 | openstackgerrit | Ghanshyam Mann proposed openstack/python-novaclient master: Microversion 2.75 - Multiple API cleanup changes https://review.opendev.org/676275 | |
| 20:39:44 | openstackgerrit | Ghanshyam Mann proposed openstack/python-novaclient master: API microversion 2.76: Add 'power-update' external event https://review.opendev.org/666792 | |
| 20:49:41 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: rt: only map compute node if we created it https://review.opendev.org/676278 | |
| 21:09:30 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: rt: only map compute node if we created it https://review.opendev.org/676280 | |
| 21:17:59 | sean-k-mooney | dansmith: technically yes just list would be fine. but on python 2.7 you would be constucting the list twice, six.move.filter is itertools.iflter on py27 and just the filter builtin on py3 so six.moves.fiter alway return an interator which we colapse into a list | |
| 21:18:31 | sean-k-mooney | we proably dont care too much for that specifc case | |
| 21:24:02 | sean-k-mooney | efried: mriedem dansmith want me to drop the use of six? or just fix typos/nits or leave it as is | |
| 21:26:16 | openstackgerrit | Merged openstack/nova stable/ocata: Replace non-nova server fault message https://review.opendev.org/674908 | |
| 21:26:23 | mriedem | sean-k-mooney: you're listifying twice on py27 anyway yeah? | |
| 21:26:27 | mriedem | so dropping six is fine | |
| 21:26:48 | sean-k-mooney | with six.moves.filter no | |
| 21:28:15 | sean-k-mooney | by that i mean it will only build the list once with six.move.filter regardless of the version of python | |
| 21:28:38 | sean-k-mooney | but i also dont partaclly care too much for this so ill do whatever ye prefer | |