search

jic-dtool / dservercore

Given a dataset UUID returns a URI to where the dataset is stored
MIT License
4 stars 5 forks source link

Flask-JWT-Extended >= 4.0 breaks server #19

Closed jotelha closed 3 years ago

jotelha commented 3 years ago

When revisiting a testing workflow that worked half a year ago and stopped working recently (https://github.com/IMTEK-Simulation/dtool-lookup-server-dependency-graph-plugin/commit/7f168491e521ade53cd6607879cb14e76a8d556c), I realized that Flask-JWT-Extended >= 4.0 introduces many breaking changes (https://flask-jwt-extended.readthedocs.io/en/stable/v4_upgrade_guide/) and leads to most of the server's tests to fail with

AssertionError: View function mapping is overwriting an existing endpoint function: dataset.wrapper

(more details at https://github.com/IMTEK-Simulation/dtool-lookup-server-dependency-graph-plugin/pull/2#issue-641883560)

To avoid looking into the issue in detail, a Flask-JWT-Extended<4.0 in the requirements should solve this for now.

Best, Johannes

tjelvar-olsson commented 3 years ago

Hi Johannes,

Thank you for reporting this. I've added a temporary fix in 4c53504

tjelvar-olsson commented 3 years ago

Fix in release: https://pypi.org/project/dtool-lookup-server/0.17.1/

Please let me know if you run into any problems. I'll look into upgrading to the latest version of the flask-jwt-extended API over the next couple of weeks.

pastewka commented 3 years ago

This seems to work, but it collides with token_generator_ldap that by default currently installs Flask-JWT-Extended 4.2. In particular, they changed the default for JWT_IDENTITY_CLAIM from identity to sub. This given a Missing claim: identity error. The fix is to either downgrad Flask-JWT-Extended for token_generator_ldap.

tjelvar-olsson commented 3 years ago

I think I have made the dtool-lookup-server code compatible with jwt-extended 4. Please have a look at: https://github.com/jic-dtool/dtool-lookup-server/tree/jwt-extended-4 and let me know what you think.

pastewka commented 3 years ago

I get the following error. It goes away if I downgrade Flask-JWT-Extended. Not sure how this is related.

dtool_lookup_server_1   | Traceback (most recent call last):
dtool_lookup_server_1   |   File "/usr/local/bin/flask", line 8, in <module>
dtool_lookup_server_1   |     sys.exit(main())
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 967, in main
dtool_lookup_server_1   |     cli.main(args=sys.argv[1:], prog_name="python -m flask" if as_module else None)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 586, in main
dtool_lookup_server_1   |     return super(FlaskGroup, self).main(*args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 782, in main
dtool_lookup_server_1   |     rv = self.invoke(ctx)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
dtool_lookup_server_1   |     return _process_result(sub_ctx.command.invoke(sub_ctx))
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
dtool_lookup_server_1   |     return _process_result(sub_ctx.command.invoke(sub_ctx))
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
dtool_lookup_server_1   |     return ctx.invoke(self.callback, **ctx.params)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 610, in invoke
dtool_lookup_server_1   |     return callback(*args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/decorators.py", line 21, in new_func
dtool_lookup_server_1   |     return f(get_current_context(), *args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 425, in decorator
dtool_lookup_server_1   |     with __ctx.ensure_object(ScriptInfo).load_app().app_context():
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 388, in load_app
dtool_lookup_server_1   |     app = locate_app(self, import_name, name)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 257, in locate_app
dtool_lookup_server_1   |     return find_best_app(script_info, module)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 83, in find_best_app
dtool_lookup_server_1   |     app = call_factory(script_info, app_factory)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 119, in call_factory
dtool_lookup_server_1   |     return app_factory()
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/dtool_lookup_server/__init__.py", line 83, in create_app
dtool_lookup_server_1   |     app.register_blueprint(bp)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 98, in wrapper_func
dtool_lookup_server_1   |     return f(self, *args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1168, in register_blueprint
dtool_lookup_server_1   |     blueprint.register(self, options, first_registration)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/blueprints.py", line 256, in register
dtool_lookup_server_1   |     deferred(state)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/blueprints.py", line 294, in <lambda>
dtool_lookup_server_1   |     self.record(lambda s: s.add_url_rule(rule, endpoint, view_func, **options))
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/blueprints.py", line 81, in add_url_rule
dtool_lookup_server_1   |     self.app.add_url_rule(
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 98, in wrapper_func
dtool_lookup_server_1   |     return f(self, *args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1282, in add_url_rule
dtool_lookup_server_1   |     raise AssertionError(
dtool_lookup_server_1   | AssertionError: View function mapping is overwriting an existing endpoint function: mongo.wrapper
tjelvar-olsson commented 3 years ago

Hi Lars, This looks like the error from before. Did you use the code from the branch jwt-extended-4? Best, Tjelvar

pastewka commented 3 years ago

I just double checked. It is installing JWT-Extended 4.2 and I am using pip install git+https://github.com/jic-dtool/dtool-lookup-server.git@jwt-extended-4 to install the development branch, still the same error. I am using the following packages (all through pip):

Jinja2-2.11.3 Mako-1.1.4 MarkupSafe-2.0.0 PyJWT-2.1.0 SQLAlchemy-1.4.15 Werkzeug-1.0.1 alembic-1.6.2 boto3-1.17.73 botocore-1.20.73 cffi-1.14.5 click-7.1.2 click-plugins-1.1.1 cryptography-3.4.7 dtool-cli-0.7.0 dtool-ecs-0.5.0 dtool-irods-0.10.1 dtool-lookup-server-0.17.1 dtool-lookup-server-dependency-graph-plugin-0.1.4 dtool-lookup-server-direct-mongo-plugin-0.1.3 dtool-lookup-server-elastic-search-plugin-0.1.dev11 dtool-s3-0.10.0 dtool-smb-0.1.0 dtoolcore-3.18.0 flask-1.1.4 flask-cors-3.0.10 flask-jwt-extended-4.2.1 flask-migrate-3.0.0 flask-pymongo-2.3.0 flask-sqlalchemy-2.5.1 greenlet-1.1.0 itsdangerous-1.1.0 jmespath-0.10.0 pyasn1-0.4.8 pycparser-2.20 pymongo-3.11.4 pysmb-1.2.6 python-dateutil-2.8.1 python-editor-1.0.4 pyyaml-5.4.1 s3transfer-0.4.2 six-1.16.0 urllib3-1.26.4
pastewka commented 3 years ago

Sorry, I am seeing now that it is reporting 0.17.1 of the lookup server. Not sure what's going on. Will double check.

pastewka commented 3 years ago

I just double checked, it is reporting 0.17.1 because the version information is hard-coded but is actually installing the latest. So this problem persists on my configuration.

tjelvar-olsson commented 3 years ago

I think this may be to do with the behaviour of pip. You already have 0.17.1 installed so it does nothing.

I'll make a new release once I have fixed: https://github.com/jic-dtool/dtool-lookup-server/issues/20

pastewka commented 3 years ago

Okay, sounds good

tjelvar-olsson commented 3 years ago

The latest release should be flask-jwt-extended 4.0 compatible: https://pypi.org/project/dtool-lookup-server/0.17.2/

pastewka commented 3 years ago

The problem persists on with 0.17.2. Let me try to debug this on my end.

jotelha commented 3 years ago

The tests work on a fresh 0.17.2 lookup server installation (following session has a mongo server running and starts in a clean venv within the current git repository root),

python -m venv ~/venv/20210518-test-python-3.8
source ~/venv/20210518-test-python-3.8/bin/activate
pip install dtool-lookup-server
pip install pytest-cov
pip freeze | tee after_pytest_cov_install.txt
pytest # OK

but fail again after installing any plugin (even if just running the plugin tests),

pip install dtool-lookup-server-direct-mongo-plugin
pip freeze | tee after_direct_mongo_plugin_intall.txt
pytest # fails as with 0.17.0

thus we will have to apply Tjelvar's server side modifications to each plugin as well.

Environment at after_pytest_cov_install.txt is

alembic==1.6.2
attrs==21.2.0
boto3==1.17.75
botocore==1.20.75
cffi==1.14.5
click==7.1.2
click-plugins==1.1.1
coverage==5.5
cryptography==3.4.7
dtool-cli==0.7.0
dtool-ecs==0.5.0
dtool-irods==0.10.1
dtool-lookup-server==0.17.2
dtool-s3==0.10.0
dtoolcore==3.18.0
Flask==1.1.4
Flask-Cors==3.0.10
Flask-JWT-Extended==4.2.1
Flask-Migrate==3.0.0
Flask-PyMongo==2.3.0
Flask-SQLAlchemy==2.5.1
greenlet==1.1.0
iniconfig==1.1.1
itsdangerous==1.1.0
Jinja2==2.11.3
jmespath==0.10.0
Mako==1.1.4
MarkupSafe==2.0.1
packaging==20.9
pluggy==0.13.1
py==1.10.0
pycparser==2.20
PyJWT==2.1.0
pymongo==3.11.4
pyparsing==2.4.7
pytest==6.2.4
pytest-cov==2.12.0
python-dateutil==2.8.1
python-editor==1.0.4
PyYAML==5.4.1
s3transfer==0.4.2
six==1.16.0
SQLAlchemy==1.4.15
toml==0.10.2
urllib3==1.26.4
Werkzeug==1.0.1

and no other modification to the environment is introduced by the plugin installation, 

$ diff after_pytest_cov_install.txt after_direct_mongo_plugin_intall.txt 
13a14
> dtool-lookup-server-direct-mongo-plugin==0.1.3
jotelha commented 3 years ago

@tjelvar-olsson looking at all the changes you have applied, will replacements like

@@ -22,7 +22,7 @@ bp = Blueprint("base_uri", __name__, url_prefix="/admin/base_uri")

 @bp.route("/register", methods=["POST"])
-@jwt_required
+@jwt_required()
 def register():
     """Register a base URI.

at all blueprint routes within the plugins resolve those AssertionError: View function mapping is overwriting an existing endpoint function: mongo.wrapper errors?

jotelha commented 3 years ago

To answer that question myself: yes, https://github.com/IMTEK-Simulation/dtool-lookup-server-direct-mongo-plugin/commit/f7ff5d448821040bc24e06b5a9e48820c7393e47

pastewka commented 3 years ago

@jotelha - this would also affect the graph plugin correct? Can you update both and make new releases?

jotelha commented 3 years ago

@jotelha - this would also affect the graph plugin correct? Can you update both and make new releases?

@pastewka already done last night

pastewka commented 3 years ago

Okay I can confirm that things now work properly with Flask-JWT-Extended>=4.0. Thanks @tjelvar-olsson @jotelha

tjelvar-olsson commented 3 years ago

I have fixed the plugin: https://pypi.org/project/dtool-lookup-server-annotation-filter-plugin/0.2.1/

tjelvar-olsson commented 3 years ago

Closing this issue now