VladaZakharova commented on PR #36052: URL: https://github.com/apache/airflow/pull/36052#issuecomment-1930129837
Hey @potiuk @vincbeck ! The latest update on this issue and the list of the errors we need to fix to be able to upgrade Connexion: 1. Based on this discussion: > > > > 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? > > > > > > > > > I am not sure this is relevant anymore. The signature of `get_api_endpoints` changed. See [here](https://github.com/apache/airflow/blob/main/airflow/providers/fab/auth_manager/fab_auth_manager.py#L149). It should simplify the problem. > > > > > > Hi @vincbeck ! I think the question here was about the line where we are appending `base_paths` variable with new registered blueprint. With the new version of Connexion the way of registering blueprints will be changed to register in connexion_app. So the method `get_api_endpoints()` with new version of Connexion will return None instead of blueprint object. So, if we can't return blueprint from the `get_api_endpoints()` method, the `base_paths` variable couldn't be extended. Should we consider still updating `base_paths` with blueprint object? Is this variable used later somewhere? The if statement here shows that we still can continue without adding blueprint to this list of base_paths. If we need this variable later, is there other way to add blueprint not using value returned by `get_api_endpoints()` since now it will return None? > > ``` > > def init_api_auth_provider(connexion_app: connexion.FlaskApp): > > """Initialize the API offered by the auth manager.""" > > auth_mgr = get_auth_manager() > > blueprint = auth_mgr.get_api_endpoints(connexion_app) > > if blueprint: > > base_paths.append(blueprint.url_prefix if blueprint.url_prefix else "") > > flask_app = connexion_app.app > > flask_app.extensions["csrf"].exempt(blueprint) > > ``` > > I think the way we should do it is really close to what you have now with few differences: > > * Rename `get_api_endpoints` to `set_api_endpoints`. The return type should be updated to `None`. Documentation should be updated as well to something like "Set API endpoint(s) definition for the auth manager.". This is a breaking change but nobody uses this interface yet, so it is a good time to do it. > * This piece of code `flask_app.extensions["csrf"].exempt(blueprint)` should be moved in the `set_api_endpoints` method using `appbuilder.app.extensions["csrf"].exempt(api.blueprint)` Blueprint is now can't be retrieved from connexion app and there will be always None. Find a way to add csrf extension to a newly created blueprint using connexion: to retrieve blueprint object from connexion_app variable to save the current logic (`flask_app.extensions["csrf"].exempt(blueprint)`) or find a way to add this extension on connexion level(check the documentation for available options). 2. Find a way to replace legacy `environ_overrides` method from old version of Connexion to fix the problem in tests `TypeError: get() got an unexpected keyword argument 'environ_overrides'`. Contact @RobbeSneyders for some help. Currently we didn't find anything in their documentation regarding new way how to override env variables. 3. 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). As a reference of existing errors please check the logs from the last build here: - In `Providers-AWS,Google` part fix problem with legacy `environ_overrides` as mentioned in step#2. https://github.com/apache/airflow/actions/runs/7462817178/job/20307226379#step:6:3267 - In `Core` part there is a failing test related to incorrect encoding in the current implementation (check method [here](https://github.com/apache/airflow/blob/main/airflow/utils/helpers.py#L243) that can be related to the error here, maybe we can change the return value from `flask` to `connexion_app`. Needs some investigation. https://github.com/apache/airflow/actions/runs/7462817178/job/20307226379#step:6:5821 - In `WWW` part fix failing tests with error `AttributeError: 'FlaskApp' object has no attribute 'test_request_context'`. Please, check the documentation [here](https://connexion.readthedocs.io/en/latest/testing.html) https://github.com/apache/airflow/actions/runs/7462817178/job/20307226379#step:6:6496 - In `CLI` part fix asserts that are related to new way of calling Gunicorn command: https://github.com/apache/airflow/actions/runs/7462817178/job/20307226379#step:6:10464 - In `Other` part fix errors connected to the incorrect header that is generated. This step requires some additional investigation, since the problem could be not connected to incorrect test, but incorrect setting of headers. https://github.com/apache/airflow/actions/runs/7462817178/job/20307226379#step:6:11744 - In `Providers-AWS` part fix the problem with `AttributeError: 'FlaskApp' object has no attribute 'test_request_context'` error. Please, check the documentation [here](https://connexion.readthedocs.io/en/latest/testing.html) https://github.com/apache/airflow/actions/runs/7462817178/job/20307226379#step:6:20406 - In `Providers-Google` part fix the problem with `AttributeError: 'Response' object has no attribute 'data'` that occurres because Response object from connexion is not the same then from flask. https://github.com/apache/airflow/actions/runs/7462817178/job/20307226379#step:6:22081 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
