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

Bump Celery to 4.2.0 #5222

Merged
merged 4 commits into from Jun 18, 2018
Merged

Bump Celery to 4.2.0 #5222

merged 4 commits into from Jun 18, 2018

Conversation

villebro
Copy link
Member

More background can be found in celery/kombu#870, Celery needs to be fixed at 4.1.0 until Kombu 4.2.2 arrives. Fixes #5221.

@codecov-io
Copy link

codecov-io commented Jun 16, 2018

Codecov Report

Merging #5222 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5222   +/-   ##
=======================================
  Coverage   63.45%   63.45%           
=======================================
  Files         261      261           
  Lines       19842    19842           
  Branches     1998     1998           
=======================================
  Hits        12591    12591           
  Misses       7242     7242           
  Partials        9        9

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b49b6c...45c49e6. Read the comment docs.

requirements.txt Outdated
@@ -17,7 +17,7 @@ geopy==1.11.0
gunicorn==19.8.0
humanize==0.5.1
idna==2.6
kombu==4.1.0
kombu<4.2
Copy link
Member

Choose a reason for hiding this comment

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

Packages in requirements.txt should be pinned to a specific version to help* ensure repeatability. Would you mind specifying a specific version.

  • Note we actually don't include all indirect requirements from the output of pip freeze as we need this to be both Python 2 and 3 compliant and thus pin the direct package dependencies defined in setup.py.

cc: @timifasubaa

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the line regarding Kombu in requirements.txt, as I checked that it is only indirectly used via Celery. In this case one can think of Celery 4.1.1 as being broken. As Kombu isn't directly specified in setup.py, leaving it out of requirements.txt feels like the better option as per the initial comment?

@john-bodley
Copy link
Member

john-bodley commented Jun 17, 2018

@villebro so for iterating on the comment. When I tested this locally (to see if we still needed to pin kombu when using celery==4.1.0),

> pyenv version
2.7.13 (set by /Users/john_bodley/.pyenv/version)
> virtualenv env
> source env/bin/activate
> (env) pip install celery==4.1.0
...
Collecting kombu<5.0,>=4.0.2 (from celery==4.1.0)
...
> (env) pip freeze | grep kombu
kombu==4.2.1

It seems like the version of kombu might still be problematic given the range versioning defined from celery==4.1.0.

@villebro
Copy link
Member Author

Oops, good call @john-bodley. I repinned Kombu at 4.1.0.

@john-bodley
Copy link
Member

Actually @villebro they've recently released celery version 4.2.0 and I was wondering whether bumping the version higher would remedy the issue. If so we should pin celery==4.2.0 in requirements.txt and require celery>=4.2.0 in setup.py to ensure we bypass this issue.

@villebro
Copy link
Member Author

Let's see what happens..

@villebro
Copy link
Member Author

Looking at CI logs bumping to 4.2.0 seems to have resolved the issue!

@john-bodley
Copy link
Member

Thanks @villebro.

@villebro villebro changed the title Downgrade celery and kombu Bump Celery to 4.2.0 Jun 17, 2018
@john-bodley
Copy link
Member

For reference here’s the item in Celery 4.2.0 changelog which remedies the issue.

@mistercrunch mistercrunch merged commit ccf2110 into apache:master Jun 18, 2018
timifasubaa pushed a commit to airbnb/superset-fork that referenced this pull request Jul 25, 2018
* Downgrade celery and kombu

* Remove kombu from requirements.txt

* Pin kombu at 4.1.0

* Bump celery to 4.2.0
john-bodley pushed a commit to john-bodley/superset that referenced this pull request Sep 12, 2018
* Downgrade celery and kombu

* Remove kombu from requirements.txt

* Pin kombu at 4.1.0

* Bump celery to 4.2.0

(cherry picked from commit ccf2110)
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Downgrade celery and kombu

* Remove kombu from requirements.txt

* Pin kombu at 4.1.0

* Bump celery to 4.2.0
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.26.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.26.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conflicting versions of celery and kombu in requirements.txt
4 participants