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

Version 4.0.0 has insufficient constraints on dependencies #1823

Closed
chrihartl opened this issue Mar 31, 2022 · 10 comments
Closed

Version 4.0.0 has insufficient constraints on dependencies #1823

chrihartl opened this issue Mar 31, 2022 · 10 comments

Comments

@chrihartl
Copy link

chrihartl commented Mar 31, 2022

Environment

Flask-Appbuilder version: 4.0.0

pip freeze output:

apispec==3.3.2
attrs==21.4.0
Babel==2.9.1
click==8.1.1
colorama==0.4.4
dnspython==2.2.1
email-validator==1.1.3
Flask==2.1.1
Flask-AppBuilder==4.0.0
Flask-Babel==2.0.0
Flask-JWT-Extended==4.3.1
Flask-Login==0.4.1
Flask-SQLAlchemy==2.5.1
Flask-WTF==0.14.3
greenlet==1.1.2
idna==3.3
importlib-metadata==4.11.3
itsdangerous==2.1.2
Jinja2==3.1.1
jsonschema==4.4.0
MarkupSafe==2.1.1
marshmallow==3.15.0
marshmallow-enum==1.5.1
marshmallow-sqlalchemy==0.26.1
packaging==21.3
pep517==0.12.0
pip-tools==6.5.1
prison==0.2.1
PyJWT==2.3.0
pyparsing==3.0.7
pyrsistent==0.18.1
python-dateutil==2.8.2
pytz==2022.1
PyYAML==6.0
six==1.16.0
SQLAlchemy==1.4.33
SQLAlchemy-Utils==0.38.2
tomli==2.0.1
Werkzeug==2.1.0
WTForms==2.3.3
zipp==3.7.0

Expected results

I would expect to be able to successfully run "import flask_appbuilder".

Actual results

An exception occurs:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\local\testfab\venv\lib\site-packages\flask_appbuilder\__init__.py", line 5, in <module>
    from .api import ModelRestApi  # noqa: F401
  File "C:\local\testfab\venv\lib\site-packages\flask_appbuilder\api\__init__.py", line 62, in <module>
    from ..security.decorators import permission_name, protect
  File "C:\local\testfab\venv\lib\site-packages\flask_appbuilder\security\decorators.py", line 21, in <module>
    from flask_login import current_user
  File "C:\local\testfab\venv\lib\site-packages\flask_login\__init__.py", line 16, in <module>
    from .login_manager import LoginManager
  File "C:\local\testfab\venv\lib\site-packages\flask_login\login_manager.py", line 24, in <module>
    from .utils import (_get_user, login_url as make_login_url, _create_identifier,
  File "C:\local\testfab\venv\lib\site-packages\flask_login\utils.py", line 13, in <module>
    from werkzeug.security import safe_str_cmp
ImportError: cannot import name 'safe_str_cmp' from 'werkzeug.security' (C:\local\testfab\venv\lib\site-packages\werkzeug\security.py)

Steps to reproduce

mkdir ~/testfab && cd ~/testfab
python -m venv venv
source venv/bin/activate
pip install flask-appbuilder==4.0.0
python -c 'import flask_appbuilder'
@chrihartl
Copy link
Author

In order to be able to install flask_appbuilder in the latest version, I need to have the following constraints in my requirements.in (or setup.py):

flask-appbuilder
werkzeug<2.1.0
jinja2<3.1.0

The constraint on werkzeug is needed due to the existing upper limit on the version of Flask-Login (>=0.3, <0.5).
The constraint on jinja2 is needed because of the existing upper limit on the version of Flask-WTF (>=0.14.2, <0.15.0).

So it looks like the setup.py of Flask-AppBuilder should have the following included in install_requires:

'werkzeug<2.1.0',
'jinja2<3.1.0',

(Here: https://github.com/dpgaspar/Flask-AppBuilder/blob/v4.0.0/setup.py#L69)

(Ideally, the upper limits on versions could be removed altogether, in the longer run...)

@azdolinski
Copy link

Werkzeug==2.1.0

Version 2.1.0 Released 2022-03-28 -> https://werkzeug.palletsprojects.com/en/2.1.x/changes/#version-2-1-0
Remove the pbkdf2_hex, pbkdf2_bin, and safe_str_cmp functions. Use equivalents in hashlib and hmac modules instead.

in this version - there has been removed 'werkzeug.security.safe_str_cmp'
you should always follow:
@ pip install -r requirements.txt

@chrihartl
Copy link
Author

@azdolinski True, the latest version of werkzeug has breaking changes, and that's why werkzeug should be constrained to <2.1.0 in the setup.py of Flask-AppBuilder, as long as Flask-AppBuilder also constrains Flask-Login to <0.5 (because these older Flask-Login versions require the older werkzeug version, but do not state this requirement explicitly in their respective setup.py install_requires statements). Similarly for jinja2 and Flask-WTF.

My wider point being that if you pip install some-package==x.y.z then you should be able to at least import stuff from that package without error (or else the package installation should have failed in the first place, namely if pip or pip-compile found that installed versions or stated version requirements cannot be reconciled with the newbie requirement). The existence of a file requirements.txt in the GitHub repository hosting the source code of that package is not relevant in this regard. A Python package should specify, what its installation requires in terms of versions of dependencies of that package. In particular, if some of those dependencies have an upper version limit then one should be aware that this can cascade into upper version limits of transitive dependencies, because there is always a chance that these transitive dependencies evolve in a breaking way.

I guess it would generally be advisible to constrain the major version number of every entry in install_requires to those values that were successfully tested. Then update early and often. In this way, a user of a package has as much freedom of choice as possible, but as much constraint as necessary, when it comes to the choice of versions of dependencies.

@barbidulle
Copy link

Hi,
The impact is the pip install flask-appbuilder does not work anymore, as it is written in the documentation (here). It would be useful to update the documentation.

Best regards

@odidev
Copy link

odidev commented Apr 12, 2022

Hi @dpgaspar,
I tried to test this package using nosetests and getting ImportError on both x86 and arm64 architectures locally or also on GHA, it seems the test cases present in utils.py are failing with the below error. Please have a look at the test output.txt

ImportError: cannot import name 'safe_str_cmp' from 'werkzeug.security'

I also worked according to the suggestions given in the comment section. I added werkzeug<2.1.0 and jinja2<3.1.0 in requirements.txt (or setup.py) file but still getting the same error on both architectures. It would be really helpful if you could share some pointers on it.

@odidev
Copy link

odidev commented Apr 26, 2022

@dpgaspar Is there any update for the above issue?

@dpgaspar
Copy link
Owner

@chrihartl Thank you for the issue,

Take a look at: #1838

@dpgaspar
Copy link
Owner

dpgaspar commented May 3, 2022

just released 4.1.0, with a fix, feel free to reopen if any problem found

@dpgaspar dpgaspar closed this as completed May 3, 2022
@chrihartl
Copy link
Author

@dpgaspar Thank you very much. I can confirm that with version 4.1.0. I can successfully run python -c 'import flask_appbuilder'.

@dpgaspar
Copy link
Owner

dpgaspar commented May 3, 2022

Thank you for reporting and taking time to help and test

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

No branches or pull requests

5 participants