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

Lots of pytest warnings / upgrade to Connexion 3 #689

Closed
osma opened this issue Apr 14, 2023 · 4 comments · Fixed by #702
Closed

Lots of pytest warnings / upgrade to Connexion 3 #689

osma opened this issue Apr 14, 2023 · 4 comments · Fixed by #702
Assignees
Milestone

Comments

@osma
Copy link
Member

osma commented Apr 14, 2023

Running pytest nowadays generates lots of warnings:

==== 384 passed, 4 skipped, 4 deselected, 807 warnings in 71.66s (0:01:11) ====

Many of these are Flask deprecation warnings triggered by connexion, e.g.:

 .../lib/python3.10/site-packages/sklearn/feature_extraction/text.py:528: UserWarning: The parameter 'token_pattern' will not be used since 'tokenizer' is not None'
    warnings.warn(

tests/test_openapi.py: 57 warnings
  .../lib/python3.10/site-packages/connexion/apis/flask_api.py:236: DeprecationWarning: '_request_ctx_stack' is deprecated and will be removed in Flask 2.3.
    setattr(flask._request_ctx_stack.top, 'connexion_context', context_dict)

tests/test_openapi.py: 57 warnings
  .../lib/python3.10/site-packages/connexion/apis/flask_api.py:236: DeprecationWarning: '_request_ctx_stack' is deprecated and will be removed in Flask 2.3. Use 'g' to store data, or 'request_ctx' to access the current context.
    setattr(flask._request_ctx_stack.top, 'connexion_context', context_dict)

tests/test_openapi.py: 60 warnings
  .../lib/python3.10/site-packages/flask/json/provider.py:188: DeprecationWarning: Setting 'json_encoder' on the app or a blueprint is deprecated and will be removed in Flask 2.3. Customize 'app.json' instead.
    warnings.warn(

tests/test_openapi.py: 60 warnings
  .../lib/python3.10/site-packages/flask/json/provider.py:230: DeprecationWarning: 'JSONEncoder' is deprecated and will be removed in Flask 2.3. Use 'Flask.json' to provide an alternate JSON implementation instead.
    return json.dumps(obj, **kwargs)

It seems to me that these have been fixed (and more) in the Connexion main branch, but the fixes are not in version 2.14.2 that we are currently using. Instead, Connexion 3.0 is coming up soon, with the most recent release being 3.0.0a4 currently.

I think we should try to upgrade to Connexion 3 because I don't think version 2 will have a long future. That should take care of most of the pytest warnings.

@juhoinkinen
Copy link
Member

I tried to upgrade to Connexion 3.0.0a6 in 79648cd but could not make the API routes work/registered: got only 404 responenses. It seemed the OpenAPI specfile (annif.yaml) was not read at all, because no error was produced even if I used a nonexistent filename in its place.

@juhoinkinen
Copy link
Member

CORS support could be added as Connexion middleware instead of using the separate flask-cors dependency, see spec-first/connexion#1660 (reply in thread).

@juhoinkinen
Copy link
Member

juhoinkinen commented May 2, 2023

I tried to upgrade to Connexion 3.0.0a6 in 79648cd but could not make the API routes work/registered: got only 404 responenses. It seemed the OpenAPI specfile (annif.yaml) was not read at all, because no error was produced even if I used a nonexistent filename in its place.

Some progress:

I noticed that "Connexion 3.0 needs to be run using an ASGI server instead of a WSGI server. While any ASGI server should work, connexion comes with uvicorn as an extra", so for the API routes to work the command to start the application is uvicorn --factory annif:create_app (using the setup in https://github.com/NatLibFi/Annif/blob/try-connexion3-alpha/annif/__init__.py).

However, using suggest method gives error Invalid Content-type (multipart/form-data), expected ['application/x-www-form-urlencoded']

Also, the annif run CLI command uses FlaskGroup, which does not about uvicorn(?), so maybe the default run command nees to be overridden to use uvicorn.

@juhoinkinen
Copy link
Member

I did not manage to make Click work with Connexion 3 the right way.

Maybe having separate create_app() and create_flask_app() functions (the former creating app with Connexion and latter with only Flask) in annif/__init__.py would help in this too. So PR #696 is better to be merged first.

@juhoinkinen juhoinkinen self-assigned this May 4, 2023
@juhoinkinen juhoinkinen modified the milestones: 1.0, Short term Aug 15, 2023
@juhoinkinen juhoinkinen modified the milestones: Short term, 1.1 Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants