airflow: Connexion 3 breaks init-views

Body

airflow/www/extensions/init_views imports connexion.decorators.validation.RequestBodyValidator connexion v3 has refactored the entire module to middleware, see: spec-first/connexion#1525 Specifically, RequestBodyValidator was removed in: spec-first/connexion#1595 The usage was added in #30596, seemingly only to override and improve the default error message. Either revert that change or find another way, preferably without using connexion internals.

Committer

  • I acknowledge that I am a maintainer/committer of the Apache Airflow project.

About this issue

  • Original URL
  • State: open
  • Created 8 months ago
  • Comments: 17 (11 by maintainers)

Most upvoted comments

The usage was added in https://github.com/apache/airflow/pull/30596, seemingly only to override and improve the default error message. Either revert that change or find another way, preferably without using connexion internals.

This was merged into Connexion in https://github.com/spec-first/connexion/pull/1761 and is already part of Connexion 3.0.0. Thanks for the contribution!


I had a quick look into how Airflow is using Connexion though, and more changes will be needed to retain the old behavior since Airflow isn’t really using the intended Interface.

Connexion 2 used to inject its functionality as decorators on the view functions, which were then registered on the underlying Flask app. There were multiple issues with this approach, which is why Connexion 3 moved most of its functionality to middleware, which is wrapped around the underlying Flask app. You can read about the design changes in more detail in this blogpost.

In itself, this doesn’t lead to any breaking changes if you’re using the intended interface of Connexion, which is an application (FlaskApp or AsyncApp in Connexion 3). However in #29631, Airflow moved away from this intended interface by creating a FlaskApi directly and registering its internal blueprint on the Flask app directly. This worked in Connexion 2, since the FlaskApi created the decorated routes. But it no longer works on Connexion 3, since it doesn’t contain the middleware, which is part of the FlaskApp.

Note that your application will still work, but it is no longer performing connexion functionality like security checks and validation. So while it might pass Airflow tests, an application built this way would fail against the connexion tests.


To migrate to Connexion 3, the changes of #29631 would have to be reverted, although you can still pass in a pre-loaded dict, which was responsible for the performance improvement mentioned.

Two other breaking changes I see are

  • You should register your error handlers on the connexion FlaskApp since the error handlers registered on the underlying flask app are ignored when running a FlaskApp. The interface is the same though, so this shouldn’t be too hard
  • You need an ASGI server to run the application. As far as I can tell, you’re using Gunicorn, which you can continue to use with a uvicorn worker.

I feel like I’m lacking context from Airflow to submit a PR on this myself, but I’m happy to collaborate and review any changes.

Hi, Connexion maintainer here 👋

Thanks for testing version 3 and submitting the PR. Let us know if you run into any other issues and we’re happy to help fix them. Your feedback is very valuable!

Connexion 3 will be out soon, we’re working on the last documentation pages as we speak (https://github.com/spec-first/connexion/issues/1531).

Connexion expressed interest to improve the error message our custom validator tries to patch, so I’ll just propose it to be included in Connexion 3 directly.

https://github.com/spec-first/connexion/issues/1152

Update: PR opened https://github.com/spec-first/connexion/pull/1761

I assume that what is urgent, is not blocking the migration path to Connexion 3 in the future, by now releasing the auth managers with an interface that expects them to return a connexion.FlaskApi.

If upgrading to Connexion 3 before that release is not an option, the best way around this from my perspective is to have the auth managers return something that can work with both Connexion 2 and Connexion 3.

  • Let the function return the specification and any kwargs to override.

    • You could provide a similar class yourself, possibly with only limited options that you want the function to be able to override. This way you’re not exposing anything Connexion-specific to the function.

Auth manager interface is not public, yet. It will take some time before having the interface considered as public so we will be able to change the interface

Also after this comment @vincbeck @jedcunningham - I think we should re-evaluate the result of our discussion whether Auth Mnagers should return FlaskApi from #34349 - because that might limit our abillity to move to Connexion 3 when we introduce Auth Managers, and we certainly would not want this.

I agree. But what is the plan? Are we going to create Flask application through connexion now as suggested by @RobbeSneyders connexion.FlaskApp? If yes then we can use connexion_app.add_api(spec, **kwargs)

I do not think we have a plan yet, but it looks like yes, we should be doing that - maybe even before we release 2.8

I assume that it should be done as soon as possible because it blocks upgrade of Werkzeug up to version 3.0.1 that patches vulnerability issue. The upgrade of Werkzeug requires an upgrade of Connexion up to version 3.

Also after this comment @vincbeck @jedcunningham - I think we should re-evaluate the result of our discussion whether Auth Mnagers should return FlaskApi from #34349 - because that might limit our abillity to move to Connexion 3 when we introduce Auth Managers, and we certainly would not want this.

I agree. But what is the plan? Are we going to create Flask application through connexion now as suggested by @RobbeSneyders connexion.FlaskApp? If yes then we can use connexion_app.add_api(spec, **kwargs)

I do not think we have a plan yet, but it looks like yes, we should be doing that - maybe even before we release 2.8

Yes ideally, the following logic would be followed:

  1. The app is instantiated using Connexion. Using a Flask app with Connexion APIs instead will lose most of Connexion’s functionality as mentioned above.

    connexion_app = connexion.FlaskApp(__name__, **kwargs)
    

    The supported kwargs here are the same as the ones on the FlaskApi class or add_api() method. This could simplify things as you’re currently setting the same kwargs on multiple FlaskApis in different places. You can still override app level kwargs when registering an api if needed.

  2. You can access the underlying Flask app as before, which can be useful to set Flask-specific configuration.

    flask_app = connexion_app.app
    
  3. Adding Apis / Blueprints must happen via the add_api() method of the Connexion app. This now does a lot more than just creating a FlaskApi.

    connexion_app = connexion_app.add_api(spec, **kwargs)
    

    This means that everywhere you’re now returning the FlaskApi, you should either:

    • Pass in the connexion_app and let the function register the api.
    • Let the function return the specification and any kwargs to override.
      • We do have an API class in connexion which can be used to represent this information. I would be willing to expose a proper interface to use it.
      • You could provide a similar class yourself, possibly with only limited options that you want the function to be able to override. This way you’re not exposing anything Connexion-specific to the function.
  4. Adding error handlers must happen via the add_error_handler() method of the Connexion app. This method has the same interface as the one from Flask. But this means you need to have access to the connexion_app instead of the flask_app here.

  5. You should run the Connexion app instead of the Flask app, using an ASGI server. If you run the Flask app directly, the Connexion middleware would not be called.

    $ gunicorn -k uvicorn.workers.UvicornWorker run:connexion_app
    

Let me know if any of these points would be blocking, and we can look for solutions together.