Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-13
19:40:56 openstackgerrit Matthew Booth proposed openstack/nova master: Add InstanceMappingList.get_by_instance_uuids https://review.openstack.org/445139
19:40:57 openstackgerrit Matthew Booth proposed openstack/nova master: Make server_external_events use an optimal number of db queries https://review.openstack.org/445142
19:40:57 openstackgerrit Matthew Booth proposed openstack/nova master: Add InstanceList.get_by_instance_mapping_list() https://review.openstack.org/445141
19:42:09 mdbooth dansmith: I was playing with the cell context thing and came up with this series ^^^
19:42:12 sean-k-mooney sdague: it addresses a sidefect of https://review.openstack.org/#/c/435575/ which due to the fact that devstack installes test-requirements globally effect the test cases of ovs in our ci and any other pep8 tests that are not run in a virtual envirionment
19:43:03 mdbooth dansmith: It only affects server_external_events, but the idea's there.
19:46:33 openstackgerrit Anusha Unnam proposed openstack/nova master: Add keypairs field to InstancePayload https://review.openstack.org/419730
20:04:19 sdague sean-k-mooney: wait, this is all to include flake8-import-order?
20:04:28 sdague can we just revert that instead?
20:05:16 sdague sean-k-mooney: or more importantly, please explain to me the actual issue that's currently happening, how we got there, and why this is the fix?
20:06:17 sean-k-mooney sdague: well the flake8-import-order change is what broke use but if your are running tempest test only there is noe reasong to install all the test requirements
20:06:30 sean-k-mooney sdague: so this change also speed up our test runs
20:06:59 sdague so, why did flake8-import-order break you?
20:07:23 sean-k-mooney sdague: but basically when flake8-import-order was added to the test requirements devstack installes it systemwide. flake8-import-order enables it test by default
20:07:42 sdague sean-k-mooney: the thing we've been here before, it used to be conditional
20:08:01 sdague ok, so I think that's actually a really good reason to remove flake8-import-order from global requirements
20:08:04 sdague and not allow it
20:08:12 sean-k-mooney when we build ovs from source to test ovs-dpdk there makefile enable pep testing if flake8 is installed
20:08:51 sean-k-mooney ovs does not enforce ordering so it cause the make command to fail and then we broke
20:08:57 sean-k-mooney sdague: well that is one option
20:09:13 sdague right, flake8-import-order seems to be far more intrusive to everything than previously realized
20:09:21 sdague and I think the answer is just remove it
20:09:45 sean-k-mooney but why is devstack installing the test dependecies globally?
20:09:51 sdague because everyone wanted that
20:09:52 sean-k-mooney that seams just wronge
20:10:08 sdague because they wanted devstack to be setting up a dev / test environment
20:10:09 sean-k-mooney what why is that better then leaving tox install them
20:10:43 sdague we can go dig up that change, the code used to be with a conditional here
20:10:55 sean-k-mooney yes and when i use devstack to set up a test environment i always use tox to run my test and i creats a python virtual enve and installs the test requirement in it
20:11:13 sdague yeh, and some of those tox environments use system libs
20:11:37 sean-k-mooney sdague: ok but is there any reson to not make it condional and default to true as we proposed
20:11:54 sdague because it's going to be another odd point of difference here
20:12:16 sdague and every one of these end up causing confusion
20:12:24 sean-k-mooney yes but devstack is breaking the seperation of test and real requirements
20:12:55 sdague sean-k-mooney: there are a bunch of different things that are bleeding across, as far as I can tell
20:13:17 sdague flake8-import-order enables true by default, which is unexpected
20:13:42 sdague ovs-dpdk is running pep8 without isolation to a venv, so it gets the impact of systems
20:13:49 sdague system installed packages
20:14:08 sdague devstack is installing test-requirements system level because that is what people previously wanted
20:14:46 sdague sean-k-mooney: you will also find that many projects use test-requirements to specify optional dependencies
20:15:04 sean-k-mooney sdague: yep
20:15:13 sdague like all the db drivers are in the nova test-requirements
20:15:30 sean-k-mooney that is why installing test requiremetns is a bad thing in my view as we neever test without them currently
20:15:36 edleafe bauzas: if a compute node goes offline, where is that caught? Currently we don't have any way of removing its resource provider information in that event.
20:15:42 sdague If you would like to get everything working with test-requirements not installed at all, I'm good with that
20:15:46 sdague I don't want a switch there
20:16:29 sdague because that is just, my 3rd party CI can just barely work based on the contents in different requirements files
20:16:54 sdague and something else will change, and break you
20:17:06 sean-k-mooney well devstack installs fine with the switch set to false and teh tempest squite works. im not sure about none tempets jobs though
20:17:34 sean-k-mooney sdague: well neutron are build ovs from source for their fuctional and fullstak jobs currently.
20:17:38 sdague sean-k-mooney: in some configuration you happen to have
20:18:02 sean-k-mooney sdague: they dont deploy nova for those but it would have broke them if they did when flake8-import-order was introduced in nova
20:18:09 sdague sean-k-mooney: sure, and there are 2 other options here. One is deal with the fact that flake8-import-order is really pretty suplerfuous
20:18:32 sdague the other is get ovs-dpdk to run it's pep8 during build under isolation
20:19:17 sean-k-mooney well that would require chaging upstream ovs makefile which we coudl explore but it would be up to them to accept that change
20:19:43 sdague sean-k-mooney: or create and activate a venv before running it
20:20:23 sean-k-mooney sdague: so activate venv and run make as part of our devstack plugin then deactivte
20:20:37 sdague sean-k-mooney: sure, if that works for the isolation
20:21:01 sean-k-mooney sdague: thats an option that i had not taught of. thats certenly possibel.
20:21:10 sdague I also, honestly think that flake8-import-order being installed globally seems weird just in general due to fallout
20:21:31 efried I just joined, so I don't have the scrollback, but +1 to that.
20:21:41 sean-k-mooney sdague: yes but to me that is a direct resutl of the devstack behavor
20:21:52 sdague and I would tend to pull that from global requirements, as I think that's going to have knock on break effects for lots of people that flake8 from the system (which is not an unreasonable thing to want to do)
20:22:13 sean-k-mooney sdague: generally test-requireemnt.txt are not expected to be installed systemwide or at least i never assume they will be
20:22:40 sdague sean-k-mooney: I accept that if you make it so that we never install them
20:23:22 sdague I don't want to be in a weird middle ground where we have to explain to devs... "oh, well for that use case you have to set this to TRUE, but to get this other thing to work, it has to be FALSE"
20:24:12 sean-k-mooney sdague: ill look into the vnev to wrap make, but would you be open to a patch to never install test requirement in devstack then without the conditional
20:24:37 sdague sean-k-mooney: yep, if you can validate that it's going to work for people
20:26:21 sdague git log -S TESTONLY
20:26:36 sdague will give you some of the change history about when this was conditional
20:27:03 sean-k-mooney well i can ask on the mailing list if people have objection to that change and we could try running the exsitng gate jobs for several projects with the proposed patch and see if they break?
20:27:17 sean-k-mooney git log -S TESTONLY ? in what repo
20:27:32 sdague sean-k-mooney: well, the issue is also for developer use case, not just the gate case
20:27:37 sdague sean-k-mooney: devstack repo
20:28:40 sean-k-mooney ah yes but again when im doing upstream dev i use devstack to set up my enviornment but i have always used the tox workflow so im not sure how many people expect them to be system wide
20:28:58 sean-k-mooney but that is a vail point
20:29:05 sean-k-mooney it has to work for both usecases
20:33:03 sean-k-mooney sdague: any thanks for taking a look at this. ill feedback you suggestion to the rest of my team and we will see if the venv change works. if not we will procuse this as an addtional path. as for flake8-import-order are you going to propose a revert or let it stand for now
20:33:55 sean-k-mooney *anyway
20:34:57 openstackgerrit STEW TY proposed openstack/nova master: Transform instance.resize_revert notification https://review.openstack.org/396811
20:39:12 rfolco cdent, around?
20:39:26 cdent yeah rfolco, for a little while longer, what's up?
20:39:47 sdague sean-k-mooney: I'll +2 a removal of it if you put it up
20:39:59 sdague we can leave the import reordering, just pull the install of it
20:40:09 rfolco cdent, quick question: how to make tox -epy27 run my new created lass under test_microversions.py? where am I noob'ing ?
20:40:24 sdague sean-k-mooney: then look at removing it from global-requirements
20:40:49 edleafe rfolco: what command are you running now?
20:40:54 cdent rfolco: you mean running just one test?
20:40:55 rfolco tox -epy27 test_microversion.TestMicroversionUtility
20:41:08 rfolco its ignoring my class
20:41:11 sean-k-mooney sdague: of flake8-import-order, ok i can do that tomorow, i am just wrapping up for the evening so ill add you to the review when its up.
20:41:39 sean-k-mooney sdague: and ya i did not want to revert the whole patch as everything else looked good to me.
20:41:54 cdent rfolco: if you do 'tox -epy27 test_microversion' does it get all the tests in the file?
20:42:11 rfolco all them, except mine :)
20:42:36 edleafe rfolco: did you save your changes? :)
20:42:42 cdent rfolco: what's the name of your test method(s)/ in the class?
20:43:21 rfolco class TestMicroversionUtility(test.NoDBTestCase): .... and def raise_405_out_of_date_version(self):
20:43:35 cdent s/raise/test_raise/
20:43:44 edleafe rfolco: test names have to start with 'test_'
20:43:47 rfolco oh boy

Earlier   Later