Update tornado requirement for new enough python versions #2761
Conversation
There was a problem hiding this comment.
Please fix failing flake8 tests
@honnix Are you ok with this split requirement based on Python version?
setup.py
Outdated
| # versions of python (3.4+ and 2.7.9+). | ||
| if sys.version_info[:2] >= (3, 4) or (sys.version_info[:2] == (2, 7) | ||
| and sys.version_info[2]>=9): | ||
| install_requires.append('tornado>=4.0,<6') |
There was a problem hiding this comment.
wouldn't you want this one to be between 5 and 6?
There was a problem hiding this comment.
I assumed that keeping it at 4.0 as minimum requirement is the correct thing to do as luigi does work with tornado 4.0 and I didn't want to introduce any possible problems that the minimum version is different depending on the python version.
Increasing the minimum version only would make sense if luigi would start using features from tornado 5 which will be complicated due to the split requirement.
There was a problem hiding this comment.
That's fair. I'm ok with that.
There was a problem hiding this comment.
@dlstadther Sorry for replying late. I was on a long vacation. Yes I'm OK with this approach.
|
Would it be possible to also support tornado 6? |
|
@tomzx Tornado 6 notes lack of support for Python 2.7 and 3.4 So the addition of Tornado 6 support would require another conditional check for Python version >= Python 3.5.2 Additionally, you'd need to verify no functional support has been removed for how Luigi calls Tornado. |
|
Hi everyone, That being said I'm not entirely sure how this specific conditional would translate into environment markers '^^. Maybe something like: |
|
@honnix sounds like a reasonable compromise to me yes. |
|
@Timost Thanks for understanding. @dlstadther Do you agree to merge? |
|
I agree |
- Added jupyterlab (#44) - Added gcc (linux) + clang (OSX) which are required by conda to compile packages w/ C extensions - Constrained tornado requirement to stop luigi breaking jupyter (see spotify/luigi#2761)
- Added jupyterlab (Closes #44) - Added gcc (linux) + clang (OSX) which are required by conda to compile packages w/ C extensions - Constrained tornado requirement to stop luigi breaking jupyter (see spotify/luigi#2761)
Description
Increase the tornado version requirement to allow tornado 5 but only on systems with python 3.4+ or 2.7.9+
This is a replacement for #2735
Motivation and Context
This has been tried before for all supported python version in #2490 but had to be reverted in #2503 due to incompatibilities in the ssl module for old python versions.
However the latest version of jupyter notebook (starting with 6.0) now requires tornado >=5 so it is no longer possible to have one consistent stack with both jupyter and luigi.
Have you tested this? If so, how?
same changes as tested in #2490 but no additional tests by me