opendevreview | Merged openstack/nova stable/ussuri: Fix request path to query a resource provider by uuid https://review.opendev.org/c/openstack/nova/+/805004 | 00:01 |
---|---|---|
melwitt | stephenfin: I noticed that when I try to run a subset of unit tests for db like "tox -epy38 nova.tests.unit.db" the tests fail with "oslo_db.sqlalchemy.enginefacade.AlreadyStartedError: this TransactionFactory is already started". I assume it's related to the db migration stuff but I don't understand how. any ideas? | 00:31 |
sean-k-mooney[m] | isnt that the error we fixed in the past with the run once decorator | 00:32 |
sean-k-mooney[m] | when it was reinitalised after SIG_HUP | 00:32 |
sean-k-mooney[m] | im not really sure why you would see that in the unit tests however | 00:33 |
melwitt | sean-k-mooney[m]: yeah but this is unit tests? when you run all tests everything passes, when you run only nova.tests.unit.db you get 70+ failures ¯\_(ツ)_/¯ | 00:34 |
sean-k-mooney[m] | melwitt regarding self +1 ill check i think it might be possible to do yes by defining on +0 as allowed for the owner | 00:34 |
sean-k-mooney[m] | so the owner of the patch would be allowed to clear the review priorty but not set it to +1 or +2 | 00:35 |
melwitt | sean-k-mooney[m]: ack. I brought it up as a possible option for making sure the +1 doesn't get used as a "ping" since some were concerned about that | 00:35 |
sean-k-mooney[m] | ill see if i can create a draft patch tomorow and include that and we can discuss it in the review | 00:36 |
melwitt | cool | 00:37 |
sean-k-mooney[m] | regarding the db failure i have not run them in a few days but i can try it in the morning | 00:37 |
sean-k-mooney[m] | is it only when you filter | 00:37 |
sean-k-mooney[m] | or do they fail if you run all them | 00:37 |
melwitt | yes only when filtering | 00:37 |
sean-k-mooney[m] | ok that is odd | 00:37 |
melwitt | everything passes if you run all | 00:37 |
sean-k-mooney[m] | im not sure why filtering would break them | 00:37 |
melwitt | me neither. it's weird | 00:38 |
sean-k-mooney[m] | maybe stephenfin will spot something | 00:38 |
melwitt | it might only be just me but I tried wiping everything, upgrading tox, re-cloning the nova repo | 00:39 |
sean-k-mooney[m] | i wonder if there was an sqlalchemy release or soemthing that has chnaged behaivor | 00:39 |
sean-k-mooney[m] | so ya just quickly ssh to my home server cloned nova and ran the db test and ya it failed for me too | 00:52 |
sean-k-mooney[m] | i only got 16 failure but it proably not deterministic | 00:53 |
sean-k-mooney[m] | its likely differnt based on the number of cores/parallel tests we have | 00:53 |
sean-k-mooney[m] | the server i ran it on had 48 threads so there will be less tests running in the same process since it will use more of them | 00:53 |
melwitt | oh, yeah ok | 00:54 |
sean-k-mooney[m] | looks like its coming direclty form the db fixture which is od | 00:55 |
sean-k-mooney[m] | *odd | 00:55 |
melwitt | yeah | 00:55 |
melwitt | I have run tests like this subset a lot of times in the past and this is the first time I see it fail like this. so I figure it must be a fairly recent change | 00:56 |
sean-k-mooney[m] | im wondering if its a recent nova change or oslo.db or sqlalchmy version change | 00:56 |
melwitt | yeah I don't find sqlalchemy in my tox env which ... I don't understand | 00:58 |
sean-k-mooney[m] | i have 1.4. something | 01:01 |
sean-k-mooney[m] | i downgraded its uppercase in pip freeze | 01:02 |
sean-k-mooney[m] | but downgrading it and oslo.db had no reall effect so i looks like that is not the issue | 01:02 |
sean-k-mooney[m] | or rather it happens with oslo.db 11 and 10 and sqlalchmey 1.4.x and 1.3.x | 01:03 |
sean-k-mooney[m] | so ya maybe a recent nova change | 01:03 |
sean-k-mooney[m] | ill try and take a look again in the morning. | 01:04 |
melwitt | oh derp | 01:04 |
melwitt | haha thanks | 01:04 |
sean-k-mooney[m] | night all o/ | 01:04 |
melwitt | SQLAlchemy==1.4.25 | 01:04 |
melwitt | gnite o/ | 01:04 |
bauzas | good morning Nova | 06:13 |
opendevreview | Pierre-Samuel Le Stang proposed openstack/nova master: Fix instance's image_ref lost on failed unshelving https://review.opendev.org/c/openstack/nova/+/807551 | 07:13 |
gibi | morning | 08:30 |
gibi | bauzas: do we start with the tox.ini python version pinning topic today at 13:00 UTC? | 09:33 |
bauzas | I was thinking so | 09:33 |
bauzas | gibi: updated the etherpad to make it clear where we start | 09:36 |
gibi | bauzas: thanks, I will gather my thoughts then | 09:36 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Query ports with admin client to get resource_request https://review.opendev.org/c/openstack/nova/+/811396 | 09:41 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Enable min pps tempest testing in nova-next https://review.opendev.org/c/openstack/nova/+/811748 | 09:41 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Avoid unbound instance_uuid var during delete https://review.opendev.org/c/openstack/nova/+/805605 | 09:48 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [nova-manage]support extended resource request https://review.opendev.org/c/openstack/nova/+/802060 | 09:48 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Reno for qos-minimum-guaranteed-packet-rate https://review.opendev.org/c/openstack/nova/+/805046 | 09:48 |
gibi | sean-k-mooney[m]: btw, there is a settings in gerrit "Set new changes to "work in progress" by default" so the gerrit dev also has similar thinking as you, that we need a separate way to signal when a patch is ready and it is not the push | 09:56 |
opendevreview | Balazs Gibizer proposed openstack/nova master: DNM: turn off two unit test cases https://review.opendev.org/c/openstack/nova/+/814735 | 10:23 |
gibi | sean-k-mooney[m], melwitt: I found the two offending unit test case that causes .AlreadyStartedError error if the db tests are run selectively ^^ | 10:24 |
gibi | but I don't know why they are breaking our test runs | 10:24 |
gibi | more funnier. I can fix it, but I don't know why the fix helps :D | 10:31 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Fix db migration unit test https://review.opendev.org/c/openstack/nova/+/814735 | 10:37 |
gibi | sean-k-mooney[m], melwitt: after this ^^ I cannot reproduce the problem locally any more. But I need somebody to help with explainig why this helps | 10:38 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Fix db migration unit test https://review.opendev.org/c/openstack/nova/+/814735 | 10:40 |
stephenfin | gibi: gdi, good find | 10:59 |
stephenfin | all that context manager stuff is global. I suspect we're doing something somewhere to break this global-ness but not when we only run those tests | 11:02 |
stephenfin | https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/test_wsgi_app.py#L71-L77 | 11:04 |
opendevreview | Wenping Song proposed openstack/nova master: Report gpu devices that only configured https://review.opendev.org/c/openstack/nova/+/814743 | 11:35 |
sean-k-mooney | gibi: the engie facade mocking is similar to what we did in https://review.opendev.org/c/openstack/nova/+/805663/9/nova/tests/unit/db/test_migration.py | 11:57 |
sean-k-mooney | gibi: in that case we needed to create our own db instance in the test to allow the url to be modifed since that is read only when the engin is created | 12:00 |
sean-k-mooney | well configured | 12:00 |
sean-k-mooney | so it does kind of make sense to me that any test that do manual db sync might need this type of mocking but i would not exepct the db syncy to actill try to restart the engin facade | 12:02 |
sean-k-mooney | stephenfin: while your looking at gate issues https://review.opendev.org/c/openstack/nova/+/814036 also ill joing the ptg shortly | 12:07 |
sean-k-mooney | oh right we are currently utc +1 | 12:13 |
opendevreview | Wenping Song proposed openstack/nova master: Cleanup guest process when vm evacuate failed and then deleted https://review.opendev.org/c/openstack/nova/+/814746 | 12:15 |
stephenfin | sean-k-mooney: I don't think it's trying to start it. I think it's trying to _configure_ it | 12:15 |
stephenfin | bauzas: I'll miss the first 30 minutes of the PTG, unfortunately | 12:16 |
bauzas | stephenfin: OK, then I'll flip some topics | 12:16 |
bauzas | stephenfin: we'll start with gibi's topic about the tox usage | 12:17 |
sean-k-mooney | well i think stephen is interested in that | 12:17 |
stephenfin | yeah, I was going to say. I can miss it though. I think a few people understand the problem so I'm sure you'll settle on something sensible | 12:18 |
sean-k-mooney | stephenfin: im hoping py39 will become a required testing env for yoga | 12:18 |
sean-k-mooney | just an fyi so we will need to be able to test with that locally but ya we understand what you were tryign to do | 12:20 |
gibi | stephenfin: ok, so we have global context_manager objects one for the api db and one for the main db. So that is what gets dirty and leaking between tests | 12:22 |
gibi | do we have some special handling for them in a fixture we use for every test case so in general test cases are not interacting? And don't we use the same fixture in the db_sync tests for a reason? | 12:23 |
stephenfin | gibi: Sigh, all the DB tests are using NoDBTestCase, so we're not actually using the Database fixture https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L611-L617 | 12:28 |
stephenfin | I still don't know why running other tests first fixes the issue though... | 12:29 |
gibi | hm, so the normal way to deal with these globals is the Database fixture but for explicit DB testing we dont use that fixture | 12:30 |
stephenfin | that's my understanding from a quick inspection, but I'm juggling many things today and haven't had time to investigate it properly yet /o\ | 12:32 |
stephenfin | it does seem strange that DB tests would not use the DB fixture though | 12:32 |
gibi | I think we have a poison fixture that prevent us touching the DB from a NoDBTestCase but it does not hit for the db_sync tests | 12:32 |
stephenfin | hmm, true | 12:33 |
gibi | I'm lost in oslo_db / sqla I guess our poision specific for the way nova objects are accessing the db and it still allows our direct get_engine calls | 12:39 |
bauzas | reminder : nova sessions start in 10 mins | 12:50 |
* gibi fetches coffee | 12:54 | |
bauzas | sessions start by now :) | 13:02 |
bauzas | if people wanna come => https://www.openstack.org/ptg/rooms/newton | 13:03 |
* stephenfin joins late | 13:53 | |
stephenfin | I assume we're on a break? | 13:53 |
gibi | yepp | 13:53 |
gibi | stephenfin: you joined for the break :D | 13:54 |
stephenfin | classic me | 13:54 |
gibi | :) | 13:54 |
bauzas | stephenfin: we basically NACK'd all your ideas while you were gone :p | 13:55 |
bauzas | terrible me | 13:55 |
bauzas | belmoreira: we have an issue with the interop session being discussed one hour later that conflicts the vncproxy discussion we were planning to have at 15:40pm | 13:58 |
bauzas | belmoreira: can we make the discussion start at 4pm ? | 13:58 |
bauzas | dang | 13:58 |
bauzas | belmoreira: I meant, can we discuss the vncproxy thing at 3pm ? | 13:59 |
bauzas | (UTC) | 13:59 |
stephenfin | bauzas: just another PTG so | 13:59 |
stephenfin | :) | 13:59 |
belmoreira | bauzas 3pm utc works for me. thanks | 13:59 |
bauzas | just hope melwitt can be present at this time | 14:00 |
bauzas | belmoreira: ack, will flip the sessions | 14:00 |
bauzas | stephenfin: we'll do neutron things until 3pm UTC | 14:11 |
*** ganso_ is now known as ganso | 14:24 | |
*** vishalmanchanda_ is now known as vishalmanchanda | 14:24 | |
*** andrewbonney_ is now known as andrewbonney | 14:26 | |
*** carloss_ is now known as carloss | 15:00 | |
bauzas | dansmith: are you available ? belmoreira is discussing about moving instances between projects and we're talking of os-chown | 15:07 |
dansmith | bauzas: not right this moment, but I can try to join soon | 15:08 |
bauzas | dansmith: cool, appreciated. | 15:08 |
dansmith | bauzas: actually, this next topic is something I have to stay for, so it'll be a bit yet | 15:11 |
bauzas | ok | 15:11 |
bauzas | no worries | 15:11 |
dansmith | bauzas: I'm here for 15 mins | 15:14 |
bauzas | ta | 15:14 |
dansmith | which line is the vnc proxy thing? | 15:17 |
stephenfin | dansmith: 394 | 15:17 |
dansmith | ah, in the "after 3pm" thing | 15:18 |
stephenfin | yeah, I've lost track of how we're picking stuff/what we've covered :) | 15:18 |
bauzas | side note: Jabra Evolve 2 65 are terrible headsets for a day long wearing them | 15:58 |
bauzas | my ears are pretty done with it | 15:58 |
bauzas | stephenfin: yeah, we cherry-picked topics | 15:58 |
bauzas | I don't wanna move topics too much as it's also confusing | 15:59 |
bauzas | brinzhang: when you're up, tell me when you want to discuss the cyborg suspend/resume spec ? we can do this either this Thursday 1pm or Friday 1pm if that suits your TZ | 17:26 |
bauzas | for the moment, placing it at the top of the discussions we have left so it would be discuss this at 1pm, but we can punt it for later | 17:27 |
bauzas | to nova folks, I flipped topics so the agenda should reflect the first topics to discuss, starting at L357 | 17:28 |
* bauzas calls it a productive day | 17:29 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!