Skip to content
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

[AIRFLOW-526] pin all dependencies in setup.py #1809

Closed
wants to merge 6 commits into from

Conversation

@mistercrunch
Copy link
Contributor

commented Sep 22, 2016

Pinning all dependencies in setup.py will help having more homogenous
installs across environments. The current approach of allowing version
ranges based on version notation is risky at best and results in bad
surprises.

The downside is that we won't get positive bug fixes and security fixes
for free, but these come at a high cost where a single bad version of a
package may take your Airflow environment down, without warning.

[AIRFLOW-526] pin all dependencies in setup.py
Pinning all dependencies in setup.py with help having more homogenous
installs across environments. The current approach of allowing version
ranges based on version notation is risky at best and results in bad
surprises.

The downside is that we won't get positive bug fixes and security fixes
for free, but these come at a high cost where a single bad version of a
package may take your Airflow environment down, without warning.
@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Sep 22, 2016

@aoen

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2016

Couple of questions:
How was it decided which pinned versions should be used?

Could this break airflow for some users who need their own specific versions (i.e. should this change wait for Airflow 2.0)?

Was this tested?

I am a big +1 on this idea.

@bolkedebruin

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2016

I am also +1

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Sep 22, 2016

@aoen I decided on versions based on the most recent within the previously specified range, using this very useful service:
https://requires.io/github/apache/incubator-airflow/requirements/?branch=master

I ran tests locally and on Travis. I did run python setup.py install and python setup.py develop locally and everything was smooth.

@aoen

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2016

That is nifty indeed! What about the airflow 2.0 comment? I think this will likely break some airflow installations out there.

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2016

As discussed I'd say we collectively bite the bullet and get uniformity from the next version (1.8.0) forward.

@aoen

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2016

LGTM

@r39132

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2016

@mistercrunch @bolkedebruin @aoen
I tested this by running python setup.py install on a fresh virtualenv. Firstly, we still have an issue with the python daemon, which resulted in AttributeError: 'module' object has no attribute 'WriteVersionInfoCommand' when executing python-daemon-2.1.1's setup.py file on macosx.
The work-around right now is to issue a separate pip install python-daemon==2.1.1 command and then rerun python setup.py install for airflow. Do we have a better workaround?

@r39132

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2016

@mistercrunch do you want to go ahead with this? It looks like it might have stagnated. Worth going forward IMO.

@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2016

+1 Same. Got bit by a pandas release over the weekend. (0.19.1)

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2016

I forced a version of numpy in setup_requires (runs before install_requires) to fix issues with pandas installation and documented the fact that people should update to the latest pip and setuptools lib prior to installing Airflow.

@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2016

Note: I have the same issue as @r39132:

Processing python-daemon-2.1.1.tar.gz
Writing /var/folders/tc/1fml66yd3jjg_mj6z601kbcs9h5tkc/T/easy_install-gL1xA8/python-daemon-2.1.1/setup.cfg
Running python-daemon-2.1.1/setup.py -q bdist_egg --dist-dir /var/folders/tc/1fml66yd3jjg_mj6z601kbcs9h5tkc/T/easy_install-gL1xA8/python-daemon-2.1.1/egg-dist-tmp-MdxbJw
Traceback (most recent call last):
  File "setup.py", line 272, in <module>
    do_setup()
  File "setup.py", line 266, in do_setup
    'extra_clean': CleanCommand,
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/core.py", line 151, in setup
    dist.run_commands()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/install.py", line 67, in run
    self.do_egg_install()
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/install.py", line 117, in do_egg_install
    cmd.run()
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 380, in run
    self.easy_install(spec, not self.no_deps)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 610, in easy_install
    return self.install_item(None, spec, tmpdir, deps, True)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 661, in install_item
    self.process_distribution(spec, dist, deps)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 709, in process_distribution
    [requirement], self.local_index, self.easy_install
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/pkg_resources/__init__.py", line 836, in resolve
    dist = best[req.key] = env.best_match(req, ws, installer)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1081, in best_match
    return self.obtain(req, installer)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1093, in obtain
    return installer(requirement)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 629, in easy_install
    return self.install_item(spec, dist.location, tmpdir, deps)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 659, in install_item
    dists = self.install_eggs(spec, download, tmpdir)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 842, in install_eggs
    return self.build_and_install(setup_script, setup_base)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 1070, in build_and_install
    self.run_setup(setup_script, setup_base, args)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/command/easy_install.py", line 1056, in run_setup
    run_setup(setup_script, args)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 240, in run_setup
    raise
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 35, in __exit__
    self.gen.throw(type, value, traceback)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 193, in setup_context
    yield
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 35, in __exit__
    self.gen.throw(type, value, traceback)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 164, in save_modules
    saved_exc.resume()
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 139, in resume
    compat.reraise(type, exc, self._tb)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 152, in save_modules
    yield saved
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 193, in setup_context
    yield
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 237, in run_setup
    DirectorySandbox(setup_dir).run(runner)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 267, in run
    return func()
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 236, in runner
    _execfile(setup_script, ns)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/sandbox.py", line 46, in _execfile
    exec(code, globals, locals)
  File "/var/folders/tc/1fml66yd3jjg_mj6z601kbcs9h5tkc/T/easy_install-gL1xA8/python-daemon-2.1.1/setup.py", line 56, in <module>

AttributeError: 'module' object has no attribute 'WriteVersionInfoCommand'
@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2016

The commit I just added: 4b6aa14
fixes the issue.

The problem was that python-daemon's setup.py had a non-relative import version that this was conflicting with my imp.load_source call.

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2016

Note that somewhere along the way funcsigs chokes and requires a newer recent-ish pip and setuptools, and I added a note about that in the installation.rst

@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

Thanks!

@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

Without the pip and setuptools upgrade, I still get:

error: The 'python-daemon==2.1.1' distribution was not found and is required by airflow

Even after running:

 pip install --upgrade pip setuptools

I still get it.

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2016

@criccomini ok I was able to reproduce and moved python-daemon to setup_requires and things seem to work now.

@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

:( Still giving me the same thing. I'm starting to get paranoid.

chrisr-MBP-13:airflow chrisr$ virtualenv ~/.virtual-env/AIRFLOW-526
New python executable in /Users/chrisr/.virtual-env/AIRFLOW-526/bin/python
Installing setuptools, pip, wheel...done.

chrisr-MBP-13:airflow chrisr$ source ~/.virtual-env/AIRFLOW-526/bin/activate

(AIRFLOW-526)chrisr-MBP-13:airflow chrisr$ pip install --upgrade pip setuptools
Collecting pip
  Using cached pip-8.1.2-py2.py3-none-any.whl
Collecting setuptools
  Using cached setuptools-28.2.0-py2.py3-none-any.whl
Installing collected packages: pip, setuptools
  Found existing installation: pip 7.1.2
    Uninstalling pip-7.1.2:
      Successfully uninstalled pip-7.1.2
  Found existing installation: setuptools 18.2
    Uninstalling setuptools-18.2:
      Successfully uninstalled setuptools-18.2
Successfully installed pip-8.1.2 setuptools-28.2.0

(AIRFLOW-526)chrisr-MBP-13:airflow chrisr$ python setup.py develop
No handlers could be found for logger "__main__"
warning: no files found matching 'MANIFEST'
warning: no files found matching '*' under directory 'extras'
warning: no previously-included files matching '.cvsignore' found under directory '*'
warning: no previously-included files matching '*.pyc' found under directory '*'
warning: no previously-included files matching '*~' found under directory '*'
warning: no previously-included files matching '.DS_Store' found under directory '*'
zip_safe flag not set; analyzing archive contents...
docutils.parsers.rst.directives.misc: module references __file__
docutils.writers.docutils_xml: module references __path__
docutils.writers.html4css1.__init__: module references __file__
docutils.writers.latex2e.__init__: module references __file__
docutils.writers.odf_odt.__init__: module references __file__
docutils.writers.pep_html.__init__: module references __file__
docutils.writers.s5_html.__init__: module references __file__

Installed /private/var/folders/tc/1fml66yd3jjg_mj6z601kbcs9h5tkc/T/easy_install-P5NME8/python-daemon-2.1.1/.eggs/docutils-0.12-py2.7.egg
/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/dist.py:340: UserWarning: The version specified (u'UNKNOWN') is an invalid version, this may not work as expected with newer versions of setuptools, pip, and PyPI. Please see PEP 440 for more details.
  "details." % self.metadata.version

Installed /Users/chrisr/Code/airflow/.eggs/python_daemon-UNKNOWN-py2.7.egg
Traceback (most recent call last):
  File "setup.py", line 272, in <module>
    do_setup()
  File "setup.py", line 266, in do_setup
    'extra_clean': CleanCommand,
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/core.py", line 111, in setup
    _setup_distribution = dist = klass(attrs)
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/dist.py", line 315, in __init__
    self.fetch_build_eggs(attrs['setup_requires'])
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/setuptools/dist.py", line 361, in fetch_build_eggs
    replace_conflicting=True,
  File "/Users/chrisr/.virtual-env/AIRFLOW-526/lib/python2.7/site-packages/pkg_resources/__init__.py", line 856, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'python-daemon==2.1.1' distribution was not found and is required by the application
@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

Also:

$ git rev-parse HEAD
2aaa629a7122d0d0e28b44f335055b73b8b8feae

I applied the diff for this PR to it. Appears to have your latest commit in it.

$ grep -B 5 -A 5 "python-daemon==2.1.1" setup.py 
        include_package_data=True,
        zip_safe=False,
        scripts=['airflow/bin/airflow'],
        setup_requires=[
            'numpy==1.11.0',
            'python-daemon==2.1.1',
        ],
        install_requires=[
            'alembic==0.8.8',
            'croniter==0.3.12',
            'dill==0.2.5',
@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2016

was that from a fresh venv? if not I'm guessing that pip freeze | grep daemon would show something like python-daemon==UNKNOWN or something like that. A fresh venv or pip uninstall python-daemon should get you back on track.

@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

It was from a fresh venv. Completely new. The very first line is where I create it.

@criccomini

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

(AIRFLOW-526)chrisr-MBP-13:airflow chrisr$ pip uninstall python-daemon
Cannot uninstall requirement python-daemon, not installed
@r39132

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

FYI, I was able to reproduce what @criccomini saw as well.

mistercrunch added 2 commits Oct 5, 2016

@mistercrunch mistercrunch force-pushed the mistercrunch:pin_deps branch to 44611d4 Oct 5, 2016

@azban

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2016

seems like this change may be a little too rigid and increase friction in installing airflow alongside someone's existing packages. does it not make sense to assume people will follow proper patterns with major vs. minor version changes, and then become more strict on a case by case basis if we see issues with individual packages.. e.g. with pandas; flask just released a very minor backwards compatibility issue in a minor release.

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2016

We've been bit 3 times in the past by packages upgrading within the strict-ish range we had setup before (within big number release after 1.0, and within minor release before 1.0). That's not mentioning the dozens of installation, upgrades and configuration specific bugs that are hard to reproduce. In some cases you have to ask people to share the output of their pip freeze and diff it with yours and that's just not sustainable.

Maybe some packages (pandas, numpy) can be set within a range, and eventually for Airflow 2.0 we can break down the airflow package into more smaller packages with less dependencies in each.

We should pin as much as possible though.

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2016

Waiting on @bolkedebruin to confirm python-daemon 1.6.1 is ok.

@gwax

This comment has been minimized.

Copy link
Contributor

commented Oct 14, 2016

Is it our job to set other people's dependencies?

The piece that I worry about is someone else setting a requirements file with airflow and some other things and having issues controlling the specific versions of libraries used.

As a library, it should be our job to specify the minimum that we need and let our users specify the exact versions they care about.

@r39132

This comment has been minimized.

Copy link
Contributor

commented Oct 18, 2016

@mistercrunch let me know what you would like to do with this. This is an important change that I would like to see committed, so please prioritize it, else we will need to close.

@bolkedebruin

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2016

@mistercrunch 1.6.1 seems ok, however the patch does not always "work" cleanly (python setup.py install complaining):

Running funcsigs-1.0.2/setup.py -q bdist_egg --dist-dir /tmp/easy_install-FxG2xm/funcsigs-1.0.2/egg-dist-tmp-K9JK1C
/usr/lib/python2.7/dist-packages/pkg_resources/__init__.py:2512: PEP440Warning: 'python-daemon (UNKNOWN)' is being parsed as a legacy, non PEP 440, version. You may find odd behavior and sort order. In particular it will be sorted as less than 0.0. It is recommend to migrate to PEP 440 compatible versions.
  PEP440Warning,
/usr/lib/python2.7/dist-packages/pkg_resources/__init__.py:2512: PEP440Warning: 'python-apt (0.9.3.11build1)' is being parsed as a legacy, non PEP 440, version. You may find odd behavior and sort order. In particular it will be sorted as less than 0.0. It is recommend to migrate to PEP 440 compatible versions.
  PEP440Warning,
error: Setup script exited with error in funcsigs setup command: Invalid environment marker: python_version<"2.7"
@r39132

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2016

@mistercrunch Can you get this PR across the line? I've "lightened" your load by closing a bunch of your other PRs. This one is def needed.

@alexvanboxel

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2016

Can I request a bump of the google api to 1.5.4 (or shall I do it after this one is merged)

@r39132

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2016

I think someone needs to take this on - the PR is more than a month old! This is an important PR and someone needs to carry this across the line. If @mistercrunch can get to this today, I will merge it, else I'd like to see if there are other takers as he seems crunched for time!

BTW, as part of testing release candidates for 1.8, we will need to run setup.py. I have had to stop using setup.py because it's non-deterministic and has created a ton of problems for us - so much so that we are just pip installing releases now in prod and staging envs.

Before we can test 1.8 rc.x, we will need this PR to get across the line.

@IIIIllllIIIIllllIIIIllllIIIIllllIIIIll

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2016

@r39132 FWIW I agree with @gwax, I don't think we should be pinning dependencies

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Nov 1, 2016

From http://stackoverflow.com/questions/28509481/should-i-pin-my-python-dependencies-versions

You should always pin your dependencies as it increases the possibility of safe, repeatable builds, even as time passes. The pinned versions are your declaration as a package maintainer that you've verified that your code works in a given environment. This has a nice side effect of preserving your sanity as you won't be inundated with bug reports in which you have to play inspector into every package codependency and system detail.
Users can always choose to ignore the pinned dependency-versions and do so at their own risk. However, as you release new versions of your library, you should update your dependency versions to take in improvements and bug fixes.

I tend to agree but have read other articles that claim the opposite. I'm not sure what to do here. Python doesn't do nice with diamond dependencies with 2 different versions. I wish python did like npm does on that regard: building a tree where different libs can each use different versions of the same lib.

@gwax

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2016

Having done some reading and performed a bit of a survey of some other (bigish) packages, my recommendation is that we pin to a known, working state in requirements.txt for development; and not pin in setup.py for distribution; we should probably test against both.

This is based on the following findings:

ipython - not pinned: https://github.com/ipython/ipython/blob/master/setup.py#L200
pytest - not pinned: https://github.com/pytest-dev/pytest/blob/master/setup.py#L51
pylons - not pinned: https://github.com/Pylons/pyramid/blob/master/setup.py#L40
django - not pinned: https://github.com/django/django/blob/master/setup.py#L50
sympy - not pinned: https://github.com/sympy/sympy/blob/master/setup.py#L365
requests - setup.py not pinned: https://github.com/kennethreitz/requests/blob/master/setup.py#L49
requests - requirements.txt pinned: https://github.com/kennethreitz/requests/blob/master/requirements.txt#L1
boto3 - setup.py not pinned: https://github.com/boto/boto3/blob/develop/setup.py
boto3 - requirements.txt pinned: https://github.com/boto/boto3/blob/develop/requirements.txt

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented Nov 1, 2016

@gwax thanks for gathering the evidence! You have me convinced.

@IIIIllllIIIIllllIIIIllllIIIIllllIIIIll

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2016

we don't seem to have a requirements.txt?

@WouterVH

This comment has been minimized.

Copy link

commented May 8, 2018

-1000
Pinning dependencies in setup.py is not the right approach.
Pinning should happen in a requirements.txt

You can pin minimum-requirements in setup.py,
but maximum-requirements only if you know for certain that new packages are not compatible AND you are not going to fix these issues.
Always avoid exact pinning.

BTW major-version changes do not necessary imply breaking-compatibility, and minor-version do not necessary imply the opposite. It should in theory, but in practice it doesn't.

Setup.py should just list its dependencies,
it is not meant to create a reproducible deployment.

Packages listed in setup.py should never be pinned.
They do not serve the same purpose as a requirements.txt.

Pinning requirements in setup.py immediately creates a dependency-hell in larger projects,
which is the reason why am I searching this repo for a reason why this pinning happened.

@Fokko

This comment has been minimized.

Copy link
Contributor

commented May 8, 2018

@WouterVH Please look at the current setup.py. Not everything is pinned anymore.

@WouterVH

This comment has been minimized.

Copy link

commented May 8, 2018

You only need to pin minimum-versions if you know (not assume or think) that your application is not going to work with older versions of that dependency, AND you know you are not going to support those older versions (which makes perfect sense)

Similar with pinning maximum-versions.

for example: Is airflow broken when using lxml 4.0>?

We were assuming it would be broken,
and therefore starting pinning our stack on older lxml-versions, hence running into dependency-hell.
but after analysis, many of these airflow-maximum-pinnings don't make sense and airflow works fine on newer versions

If airflow would be broken, users should file a bugreport here.

When I run pipdeptree in our stack, we get these warnings about potential conflicts, but airflow runs fine:

> .../virtualenv/bin/pipdeptree
  Warning!!! Possibly conflicting dependencies found:
  * nz-airflow==1.8.1
   - future [required: <0.16,>=0.15.0, installed: 0.16.0]
   - flask-wtf [required: ==0.12, installed: 0.14.2]
   - jinja2 [required: <2.9.0,>=2.7.3, installed: 2.9.6]
   - psutil [required: >=4.2.0,<5.0.0, installed: 5.2.2]
   - flask [required: <0.12,>=0.11, installed: 0.12.2]
   - alembic [required: <0.9,>=0.8.3, installed: 0.9.5]
   - lxml [required: <4.0,>=3.6.0, installed: 4.1.1]
   - tabulate [required: >=0.7.5,<0.8.0, installed: 0.8.2]
@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

Agreed, for Apache Superset we recently changed our setup.py to only set up upper-lower bounds where absolutely needed (when we know that versions of the lib won't work for sure) and leave as much open as possible.

On top of that, we provide an optional requirements.txt that can be used as the "reference implementation" used for the builds and tests. Users can decide to use it or not. Ideally requirements.txt is generated by pip-compile and pins the whole tree.

@artwr

This comment has been minimized.

Copy link
Contributor

commented May 8, 2018

Yep, I think that's the way forward. This is how dependencies are handled in our Big Data Environment as well. Don't pin unless absolutely necessary, this way we can test with the latest, and maybe package the pinned versions with a tried and tested requirements.txt in the release.

@mistercrunch

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

Without the pip-compiled requirements.txt things can be real flaky though, the builds can start failing as soon as some lib with backward incompatible changes get pushed out.

I think that there can be some challenges around pip-compile and py2/3 . In some cases you need both a requirements.txt and requirements3.txt that differ a little.

@artwr

This comment has been minimized.

Copy link
Contributor

commented May 8, 2018

Sure, but then we get early warning that things will break, once we see the issue, we can pin and people can rebase. Java install broke recently, and that's how it was handled, I think that's fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.