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

[DRAFT] Switch to Connexion 3 framework #39055

Closed
wants to merge 1 commit into from

Commits on Jun 24, 2024

  1. Switch to Connexion 3 framework

    This is a huge PR being result of over a 100 commits
    made by a number of people in #apache#36052 and apache#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.
    
    Co-authored-by: sudipto baral <[email protected]>
    Co-authored-by: satoshi-sh <[email protected]>
    Co-authored-by: Maksim Yermakou <[email protected]>
    Co-authored-by: Ulada Zakharava <[email protected]>
    
    Better API initialization including vending of API specification.
    
    The way paths are added and initialized is better (for example
    FAB contributes their path via new method in Auth Manager.
    
    This also add back-compatibility to FAB auth manaager to continue
    working on Airflow 2.9.
    Ulada Zakharava authored and potiuk committed Jun 24, 2024
    Configuration menu
    Copy the full SHA
    9c6fb6b View commit details
    Browse the repository at this point in the history