[GitHub] [airflow] potiuk opened a new pull request #15182: Adds Blinker dependency which is missing after recent changes

[GitBox <gi...@apache.org>]

potiuk opened a new pull request #15182:
URL: https://github.com/apache/airflow/pull/15182


   This PR fixes a problem introduced by #14144
   
   This is a very weird and unforeseen issue. The change introduced a
   new import from flask `before_render_template` and this caused
   flask to require `blinker` dependency, even if it was not
   specified before as 'required' by flask. We have not seen it
   before, because changes to this part of the code do not trigger
   K8S tests, however subsequent PRs started to fail because
   the setup.py did not have `blinker` as dependency.
   
   However in CI image `blinker` was installed because it is
   needed by sentry. So the problem was only detectable in the
   production image.
   
   This is an ultimate proof that our test harness is really good in
   catchig this kind of errors.
   
   The root cause for it is described in
   https://stackoverflow.com/questions/38491075/flask-testing-signals-not-supported-error
   
   Flask support for signals is optional and it does not blinker as
   dependency, but importing some parts of flask triggers the need
   for signals.
   
   <!--
   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 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/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk merged pull request #15182: Adds Blinker dependency which is missing after recent changes

[GitBox <gi...@apache.org>]

potiuk merged pull request #15182:
URL: https://github.com/apache/airflow/pull/15182


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org