RobbeSneyders commented on issue #35234:
URL: https://github.com/apache/airflow/issues/35234#issuecomment-1793404192

   > 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](https://medium.com/p/a5dc17e81ff8#24ba).
   
   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 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.


-- 
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]

Reply via email to