New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ceph-disk: add setting for external py-modules for tox-testing #15433

Merged
1 commit merged into from Jun 8, 2017

Conversation

Projects
None yet
3 participants
@wjwithagen
Contributor

wjwithagen commented Jun 2, 2017

ceph-disk: use system modules if needed

  • prettytable usage was introduced in:
    3fa8bb1
    It is in the install-deps.sh file to be installed, but
    it is not per default installed in the testenvironment
    So allow tox tests to use "external" modules

Signed-off-by: Willem Jan Withagen wjw@digiware.nl

@ghost ghost added tests bug fix labels Jun 2, 2017

@ghost

This comment has been minimized.

ghost commented Jun 2, 2017

could you please add that to requirements-tests.txt instead ? If it's required only for tox, that should be enough.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 2, 2017

@dachary
I tried that since suggested that, but it did not seem to work.
But I'll try again.

@smithfarm smithfarm requested a review from Jun 2, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 2, 2017

@dachary
Tried it again, and it is again only added to virtualenv when it is in requirements.txt.
And when added to test-requirements.txt it is not added.

Already using interpreter /usr/local/bin/python2.7
New python executable in /tmp/ceph-disk-virtualenv/bin/python2.7
Also creating executable in /tmp/ceph-disk-virtualenv/bin/python
Installing setuptools, pip, wheel...done.
Requirement already up-to-date: pip>=6.1 in /tmp/ceph-disk-virtualenv/lib/python2.7/site-packages
Collecting tox>=1.9
  Url 'file:///home/wjw/wip/src/ceph-disk/wheelhouse' is ignored: it is neither a file nor a directory.
  Using cached tox-2.7.0-py2.py3-none-any.whl
Collecting virtualenv>=1.11.2; python_version != "3.2" (from tox>=1.9)
  Url 'file:///home/wjw/wip/src/ceph-disk/wheelhouse' is ignored: it is neither a file nor a directory.
  Using cached virtualenv-15.1.0-py2.py3-none-any.whl
Collecting py>=1.4.17 (from tox>=1.9)
  Url 'file:///home/wjw/wip/src/ceph-disk/wheelhouse' is ignored: it is neither a file nor a directory.
  Using cached py-1.4.33-py2.py3-none-any.whl
Collecting pluggy<1.0,>=0.3.0 (from tox>=1.9)
  Url 'file:///home/wjw/wip/src/ceph-disk/wheelhouse' is ignored: it is neither a file nor a directory.
  Using cached pluggy-0.4.0-py2.py3-none-any.whl
Installing collected packages: virtualenv, py, pluggy, tox
Successfully installed pluggy-0.4.0 py-1.4.33 tox-2.7.0 virtualenv-15.1.0
Collecting prettytable (from -r requirements.txt (line 1))
  Url 'file:///home/wjw/wip/src/ceph-disk/wheelhouse' is ignored: it is neither a file nor a directory.
Installing collected packages: prettytable
Successfully installed prettytable-0.7.2
Obtaining file:///usr/srcs/Ceph/work/ceph/src/ceph-disk
Requirement already satisfied: setuptools in /tmp/ceph-disk-virtualenv/lib/python2.7/site-packages (from ceph-disk==1.0.0)
Installing collected packages: ceph-disk
  Running setup.py develop for ceph-disk
Successfully installed ceph-disk
@ghost

ghost approved these changes Jun 2, 2017

@yuriw

This comment has been minimized.

Contributor

yuriw commented Jun 7, 2017

@wjwithagen suspect cause for errors during make check:

Installing collected packages: six, appdirs, pyparsing, packaging, setuptools
Found existing installation: setuptools 0.9.8
Uninstalling setuptools-0.9.8:
Successfully uninstalled setuptools-0.9.8
Successfully installed appdirs-1.4.3 packaging-16.8 pyparsing-2.2.0 setuptools-35.0.2 six-1.10.0
Collecting tox>=1.9
Collecting virtualenv>=1.11.2; python_version != "3.2" (from tox>=1.9)
Collecting pluggy<1.0,>=0.3.0 (from tox>=1.9)
Collecting py>=1.4.17 (from tox>=1.9)
Installing collected packages: virtualenv, pluggy, py, tox
Successfully installed pluggy-0.4.0 py-1.4.34 tox-2.7.0 virtualenv-15.1.0
Collecting prettytable (from -r requirements.txt (line 1))
Could not find a version that satisfies the requirement prettytable (from -r requirements.txt (line 1)) (from versions: )
No matching distribution found for prettytable (from -r requiremenInstalling collected packages: six, appdirs, pyparsing, packaging, setuptools
Found existing installation: setuptools 0.9.8
Uninstalling setuptools-0.9.8:
Successfully uninstalled setuptools-0.9.8
Successfully installed appdirs-1.4.3 packaging-16.8 pyparsing-2.2.0 setuptools-35.0.2 six-1.10.0
Collecting tox>=1.9
Collecting virtualenv>=1.11.2; python_version != "3.2" (from tox>=1.9)
Collecting pluggy<1.0,>=0.3.0 (from tox>=1.9)
Collecting py>=1.4.17 (from tox>=1.9)
Installing collected packages: virtualenv, pluggy, py, tox
Successfully installed pluggy-0.4.0 py-1.4.34 tox-2.7.0 virtualenv-15.1.0
Collecting prettytable (from -r requirements.txt (line 1))
Could not find a version that satisfies the requirement prettytable (from -r requirements.txt (line 1)) (from versions: )
No matching distribution found for prettytable (from -r requirements.txt (line 1))
make[3]: *** [src/ceph-disk/CMakeFiles/ceph-disk] Error 1
make[2]: *** [src/ceph-disk/CMakeFiles/ceph-disk.dir/all] Error 2
make[1]: *** [CMakeFiles/tests.dir/rule] Error 2
ts.txt (line 1))
make[3]: *** [src/ceph-disk/CMakeFiles/ceph-disk] Error 1
make[2]: *** [src/ceph-disk/CMakeFiles/ceph-disk.dir/all] Error 2
make[1]: *** [CMakeFiles/tests.dir/rule] Error 2

@yuriw yuriw removed the wip-yuri2-testing label Jun 7, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 7, 2017

@yuriw @dachary
'mmmm, I see the problem, but I do not understand it.

How can this test complete if tox is not able to find a matching prettytable, and it aborts under FreeBSD because it is not included in any of the required lists for tox. So I guess it is there, but cannot be found by the tox setup stage.
This is sort as where my Python knowledge is lacking.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 7, 2017

@yuriw @dachary

When I run install-deps.sh on Centos OS, it installs:

  Installing : python-prettytable-0.7.2-2.el7.centos.noarch                 4/5
  Verifying  : python-prettytable-0.7.2-2.el7.centos.noarch                 3/5
@ghost

This comment has been minimized.

ghost commented Jun 8, 2017

This indeed is a new dependency for the ceph CLI, as of 3fa8bb1. Since the ceph CLI does not provide a requirements list (it relies on packages to provide the desired python modules), it is necessary for ceph-disk to depend on prettytable.

@ghost

This comment has been minimized.

ghost commented Jun 8, 2017

@yuriw do you have the full log of the traceback you copy/pasted ? I don't understand why it would fail like that. https://pypi.python.org/pypi/PrettyTable/0.7.2

@ghost

This comment has been minimized.

ghost commented Jun 8, 2017

@wjwithagen 9e3d7d9 added a dependency to python-prettytable. Maybe you can resolve that problem by adding a dependency to the corresponding freebsd package ?

It is a little weird to add a dependency to ceph-disk that really is a dependency of the Ceph cli and if we can avoid it, that would be a good thing.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 8, 2017

@dachary
Well actually I have prettytable available in the regular python environment:
/usr/local/lib/python2.7/site-packages/prettytable-0.7.2-py2.7.egg-info

But if I do not add it to the requirements.txt file, it does not get installed in the virtualenv, and then all calls to ceph-disk fail on missing prettytable. And a lot of tests fail
So that is why we started this whole PR, which fixed it for FreeBSD, but obviously not for some of the runs in theutology...

Now running this PR on Centos, it actually does fail there too.
Adding it to test-requirements.txt does not do anything, just like on FreeBSD.

So I guess there is something fishy with the tox.ini config or the handleing of it??

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 8, 2017

@dachary

I was looking that the tox config page: https://tox.readthedocs.io/en/latest/config.html#confval-sitepackages=True|False

And found this option that allow test programs to actually use what is available outside the tox environment... And with that I can get it to work as well....
Maybe that is too dangerous?

@ghost

This comment has been minimized.

ghost commented Jun 8, 2017

Oh, you're correct, tox will not use site wide dependencies by default. Good catch. I don't think activating this option is dangerous because ceph-disk does not have any dependency anyway. Would you mind updating this PR with sitepackages=True ?

ceph-disk: use system modules if needed
 - prettytable usage was introduced in:
	3fa8bb1
   It is in the install-deps.sh file to be installed, but
   it is not per default installed in the testenvironment
   So allow tox tests to use "external" modules

Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>

@wjwithagen wjwithagen changed the title from ceph-disk: add required py-module for tox-testing to ceph-disk: add setting for external py-modules for tox-testing Jun 8, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 8, 2017

@dachary
Ping!

@ghost ghost merged commit 40e84a1 into ceph:master Jun 8, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment