Respect venv in sbin/exabgp #907
Conversation
Merge to update
- By checking if we're in a venv, reorder the python binary usage order
|
Even tho Python 2 is still failing we're back to using correct runtime: |
| for INTERPRETER in "$INTERPRETER" python3 pypy3 python pypy; do | ||
| # Prefer `python` binary when in a venv | ||
| INTS="$INTERPRETER python3 pypy3 python pyp" | ||
| if [ -d "${VIRTUAL_ENV}" ]; then |
There was a problem hiding this comment.
AFIU this should also test that we are running travis checking if ${TRAVIS} is "true".
ExaBGP 4.1 support is python3 only python2 is legacy (works but not supported anymore) and python 5 released after python2 EOL will remove six support.
There was a problem hiding this comment.
I am happy to not care about Python 2 and pypy (if that’s what you’re saying). Can I just remove Python2 and pypy from Travis? Should I add pypy3 ?
If we keep this, I don’t think we need to evaluate if we’re on Travis, activated venvs are the same on Travis and on your Mac ... and if you wanted to test python2 for some reason you’d need this (if you wanted to use sbin/exabgp). I normally just use the entry point in the venv, but the functional tests etc. Didn’t like that and I didn’t debug why.
There was a problem hiding this comment.
I want to support pypy. If possible within the 4.x releases python2 code should still work (not to cause issues with people stuck with python2 on their bistro). Testing for it is however showing problematic, but it has nothing to do with this script. It works as expected AFAICS.
When pypy is installed, python is linked to it and but python3 then does not get psutil installed, even with the latest .travis.yml file which attempt to do so !
I could re-change all the #! in etc/exabgp/run/*py to use /usr/bin/python (which would cause it to use pypy and not /usr/bin/python3 - but I am not sure it is really the best way forward.
Some people install exabgp by cloning it in /opt and then running from this location so whatever is in sbin/exabgp should be considered production code. Also the behaviour within venv, and without, should be consistent to not cause any surprises.
Please keep in mind that I am currently on holidays and performing this work between personal commitments :-)
There was a problem hiding this comment.
(This is all low pri and we can talk about it when you return to work - I just want a CI I can rely on when I do future PRs ... I like confidence the ✅)
I don't feel changing the #! is needed. exabgp should indeed default to #!/usr/bin/env python3 imo.
The problem with Travis and if people are relying on a Python 2venv today AND using sbin/exabgp, there will only be a python in the venv/bin directory. If the system (like Travis does) has a python3 binary in PATH, with your current search, it is found and selected.
Due to this, I feel CI should always run things with $INTERPRETER file.py. Then it will work, in venv or in a cloned /opt install.
psutils is always getting installed in the venv. The problem is files not using the venv interpreter. I feel we should make all files do so.
Ping me in IRC when your return and we can agree on a way to unify all this and I'm happy to do the work.
There was a problem hiding this comment.
FYI - I had missed you'd done a few commits. Looking now :)
Edit: I feel we're regressing here ... Lets chat when you're back. I've made my comments and tried to be clear.
There was a problem hiding this comment.
psutils is always getting installed in the venv. The problem is files not using the venv interpreter. I feel we should make all files do so.
I am fine with working on this during my holidays.
psutil is SUPPOSED to be installed for every python version but ALL the CI failures are due to its absence ! This is why you saw so many permutation of installation in the last commit to try to make it work!
When I enabled SSH debug, just running the install by hand fixed the issue every time ! 🤕
There was a problem hiding this comment.
It's cause the tests are not running with the venv's python interpreter and this missing psutils.
Even on the latest diff I see this. The Python 2 job is running with 3.5.2:
Output:
$ ./sbin/exabgp --version
ExaBGP : (HEAD-04056885f956e63886f0b0679c3b52550687546d
Python : 3.5.2 (default, Nov 12 2018, 13:43:14) [GCC 5.4.0 20160609]
Uname : Linux travis-job-48785847-c01a-4d89-a19e-96777a567bba 4.15.0-1028-gcp #29~16.04.1-Ubuntu SMP Tue Feb 12 16:31:10 UTC 2019 x86_64
Root : /home/travis/build/Exa-Networks/exabgp
The command "./sbin/exabgp --version" exited with 0.
- That should be 2.7.15 ...
|
Now we can debug why Python2 and pypy are failing :D I feel it will be some Python 3 thing that was added as Python 2 tests on Travis have not been ran under Python 2 for awhile. Would love to get more output from a failed functional test so it required less manual debugging when something goes wrong. |
More work for #905