potiuk opened a new pull request, #39055:
URL: https://github.com/apache/airflow/pull/39055
This is a huge PR being result of over a 100 commits made by a number of
people in ##36052 and #37638. It switches to Connexion 3 as the driving backend
implementation for both - Airflow REST APIs and Flask app that powers
Airflow UI. It should be largely
backwards compatible when it comes to behaviour of both APIs and Airflow
Webserver views, however due to decisions made by Connexion 3 maintainers, it
changes heavily the technology stack used under-the-hood:
1) Connexion App is an ASGI-compatible Open-API spec-first
framework using ASGI as an interface between webserver
and Python web application. ASGI is an asynchronous
successor of WSGI.
2) Connexion itself is using Starlette to run asynchronous
web services in Python.
3) We continue using gunicorn appliation server that still
uses WSGI standard, which means that we can continue using
Flask and we are usig standard Uvicorn ASGI webserver that
converts the ASGI interface to WSGI interface of Gunicorn
Some of the problems handled in this PR
There were two problem was with session handling:
* the get_session_cookie - did not get the right cookie - it returned
"session" string. The right fix was to change cookie_jar into cookie.jar
because this is where apparently TestClient of starlette is holding the cookies
(visible when you debug)
* The client does not accept "set_cookie" method - it accepts passing
cookies via "cookies" dictionary - this is the usual httpx client
- see https://www.starlette.io/testclient/ - so we have to set cookie
directly in the get method to try it out
Add "flask_client_with_login" for tests that neeed flask client
Some tests require functionality not available to Starlette test client as
they use Flask test client specific features - for those we have an option to
get flask test client instead of starlette one.
Fix error handling for new connection 3 approach
Error handling for Connexion 3 integration needed to be reworked.
The way it behaves is much the same as it works in main:
* for API errors - we get application/problem+json responses
* for UI erros - we have rendered views
* for redirection - we have correct location header (it's been missing)
* the api error handled was not added as available middleware in the www
tests
It should fix all test_views_base.py tests which were failing on lack of
location header for redirection.
Fix wrong response is tests_view_cluster_activity
The problem in the test was that Starlette Test Client opens a new
connection and start new session, while flask test client uses the same
database session. The test did not show data because the data was not committed
and session was not closed - which also failed sqlite local tests with
"database is locked" error.
Fix test_extra_links
The tests were failing again because the dagrun created was not committed
and session not closed. This worked with flask client that used the same
session accidentally but did not work with test client from Starlette. Also it
caused "database locked" in sqlite / local tests.
Switch to non-deprecated auth manager
Fix to test_views_log.py
This PR partially fixes sessions and request parameter for test_views_log.
Some tests are still failing but for different reasons - to be investigated.
Fix views_custom_user_views tests
The problem in those tests was that the check in security manager was based
on the assumption that the security manager was shared between the client and
test flask application - because they were coming from the same flask app. But
when we use starlette, the call goes to a new process started and the user is
deleted in the database - so the shortcut of checking the security manager did
not work.
The change is that we are now checking if the user is deleted by calling
/users/show (we need a new users READ permission for that)
- this way we go to the database and check if the user was indeed deleted.
Fix test_task_instance_endpoint tests
There were two reasons for the test failed:
* when the Job was added to task instance, the task instance was not merged
in session, which means that commit did not store the added Job
* some of the tests were expecting a call with specific session and they
failed because session was different. Replacing the session with mock.ANY tells
pytest that this parameter can be anything - we will have different session
when when the call will be made with ASGI/Starlette
Fix parameter validation
* added default value for limit parameter across the board. Connexion 3 does
not like if the parameter had no default and we had not provided one - even if
our custom decorated was adding it. Adding default value and updating our
decorator to treat None as `default` fixed a number of problems where limits
were not passed
* swapped openapi specification for /datasets/{uri} and /dataset/events.
Since `{uri}` was defined first, connection matched `events` with `{uri}` and
chose parameter definitions from `{uri}` not events
Fix test_log_enpoint tests
The problem here was that some sessions should be committed/closed but also
in order to run it standalone we wanted to create log templates in the database
- as it relied implcitly on log templates created by other tests.
Fix test_views_dagrun, test_views_tasks and test_views_log
Fixed by switching to use flask client for testing rather than starlette.
Starlette client in this case has some side effects that are also impacting
Sqlite's session being created in a different thread and deleted with
close_all_sessions fixture.
Fix test_views_dagrun
Fixed by switching to use flask client for testing rather than starlette.
Starlette client in this case has some side effects that are also impacting
Sqlite's session being created in a different thread and deleted with
close_all_sessions fixture.
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information.
In case of fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party
License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a
newsfragment file, named `{pr_number}.significant.rst` or
`{issue_number}.significant.rst`, in
[newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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]
