RobbeSneyders commented on PR #36052: URL: https://github.com/apache/airflow/pull/36052#issuecomment-1958422648
Sorry for the late response, let me try to give some pointers for the open issues. > 1. Based on this discussion: > 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). The most future-proof solution would be to move the `csfr` protection to a middleware such as [this one](https://github.com/simonw/asgi-csrf/tree/main). The `skif_if_scope` parameter seems to be a replacement for the `exempt` functionality you're using. > 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. There's no one-on-one replacement for this as far as I can tell. This is also not something Connexion can easily offer, since we just leverage the `starlette` test client. The easiest approach I can think of, is to add a middleware instead. ```python class RemoteUserMiddleware: def __init__(self, asgi_app: Callable, remote_user: str) -> None: self.asgi_app = asgi_app self.remote_user = remote_user async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: scope["REMOTE_USER"] = self.remote_user await self.asgi_app(scope, receive, send) ``` You could use this across tests like so: ```python app.add_middleware(RemoteUserMiddleware) with app.test_client(): ... ``` Note that: - This sets the `REMOTE_USER` on a test client level instead of a request level though. - You might have to update how this variable is accessed as well. `request.scope["REMOTE_USER"]` should work. > 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 See above > * 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 I think this is due to a change in Flask and a non-issue: https://github.com/pallets/flask/issues/5286 > * 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 Two possibilities here: - Use the underlying Flask app: `app.app.test_request_context()` - Switch to the Connexion test context in the reference docs, but you will also need to change the context being used in the tested code. > * 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 Looks like this just needs an update to the test code to match the renamed arguments. > * 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 I don't see how the linked issue is related to headers, but maybe I'm missing some context. The error I see is ```python AttributeError: 'FlaskApp' object has no attribute 'config' ``` Which is because you're treating the Connexion App like a Flask App. Changing `app.config` to `app.app.config` should give you what you want. > * 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 See above. > * 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 The returned response [provides a lot of methods](https://www.python-httpx.org/api/#response) to access the data. The one on one translation is probably `.content`, but if you're decoding anyway, you could use `.text` instead. Hope this helps! -- 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
