MaksYermak commented on PR #36052:
URL: https://github.com/apache/airflow/pull/36052#issuecomment-1884982252
Hello Team,
I see several issues which we need to fix for finishing this PR.
1. Fix unit tests which use `app`.
After connexion update `create_app` function returns `connexion_app` instead
of `flask_app`, but unit tests in most cases expect `flask_app`. In this case
we need to update all our unit tests (we have ~1000 tests).
List of errors which I found:
- for `test_client()` we should use `connexion_app` not `flask_app`. I
fixed this error for most tests in the last commit.
- `TypeError: get() got an unexpected keyword argument
'environ_overrides'` this error is the most frequent now. For fixing it we need
to find the equivalent for this variable `environ_overrides` in connexion's
`test_client()`. I didn't find any equivalent in Connexion and Starlette docs.
@RobbeSneyders could you help us with that?
- `AttributeError: 'Response' object has no attribute 'data'` I think
this happens because `Response` object from connexion is not the same then from
flask.
Also we have a big amount of different errors which do not have a similar
pattern.
2. Update `init_api_auth_provider` function.
In ` get_api_endpoints` function we use `connexion_app.add_api` and expect
`FlaskApi` as a return object, but `add_api` function always return `None`.
Previously we use `FlaskApi` object for taking blueprints and then extend our
`base_paths` here:
https://github.com/apache/airflow/pull/36052/files#diff-a1cf5a1eea3231a292d789d82df7943bfe8fa89ca955404b2f9cdaa25e08fa80R309
@vincbeck is it important to save this logic for fab_auth_manager or is it
enough to register blueprints under `connexion_app.add_api` functionality?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
