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

Remove the limit on Gunicorn dependency #15611

Merged
merged 1 commit into from
Apr 30, 2021

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Apr 30, 2021

It seems that the < 20.0 limit for gunicorn was added at some point
in time without actual reason. We are already using gunicorn in
1.10 line of Airflow, so it should not be a problem to bump the
version of gunicorn, especially that the 19. line is somewhat
deprecated already.

This change came after the discussion n #15570


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@potiuk potiuk requested review from kaxil and ashb April 30, 2021 08:07
It seems that the < 20.0 limit for gunicorn was added at some point
in time without actual reason. We are already using gunicorn in
1.10 line of Airflow, so it should not be a problem to bump the
version of gunicorn, especially that the 19. line is somewhat
deprecated already.

This change came after the discussion n apache#15570
@potiuk potiuk changed the title Remove the limit on Gunicorn constraint. Remove the limit on Gunicorn dependency Apr 30, 2021
@potiuk
Copy link
Member Author

potiuk commented Apr 30, 2021

Looks like all tests are passing with gunicorn 20.1.0 (the latest one). I tested it locally as well, and everything seems to be fine Airflow works with several gunicorn workers - looks like no problems. I think we can safely merge this one.

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel great about no upper limit here - it means Gunicorn could release a v22 that breaks us.

@uranusjr
Copy link
Member

That's when the constraints files is useful 🙂 Python packaging encourages the "compatible unless proven guilty" approach since it's much easier for the user to add extra constraints if gunicorn 22 proves incompatible, but it's much more difficult to remove gunicorn<22 if it causes unnecessarily conflicts.

@potiuk
Copy link
Member Author

potiuk commented Apr 30, 2021

Agree with @uranusjr - our constraints approach is designed to prevent precisely this problem and it works with most of our dependencies.

@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Apr 30, 2021
@potiuk potiuk merged commit d7a14a8 into apache:master Apr 30, 2021
@potiuk potiuk deleted the relax-gunicorn-constraint branch April 30, 2021 16:08
potiuk added a commit to potiuk/airflow that referenced this pull request May 6, 2021
It seems that the < 20.0 limit for gunicorn was added at some point
in time without actual reason. We are already using gunicorn in
1.10 line of Airflow, so it should not be a problem to bump the
version of gunicorn, especially that the 19. line is somewhat
deprecated already.

This change came after the discussion n apache#15570

(cherry picked from commit d7a14a8)
@potiuk potiuk added this to the Airflow 2.0.3 milestone May 9, 2021
@ashb ashb modified the milestones: Airflow 2.0.3, Airflow 2.1 May 18, 2021
@potiuk potiuk restored the relax-gunicorn-constraint branch April 26, 2022 20:48
@potiuk potiuk deleted the relax-gunicorn-constraint branch July 29, 2022 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants