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

ISSUE-1006: Revert #954 and update redis-py to at least 3.2.0 #1011

Merged
merged 1 commit into from
Feb 22, 2019

Conversation

lithammer
Copy link
Contributor

@lithammer lithammer commented Feb 21, 2019

Solution proposal 1 to fix #1006. See #1012 proposal 2.

This reverts #954, and bumps the required redis-py dependency to 3.2.0 to include this fix: redis/redis-py@4e1e748

Fixes #1006

This reverts celery#954, and bumps the required redis-py dependency to 3.2.0
to include this fix:

redis/redis-py@4e1e748

Fixes celery#1006
@lithammer lithammer changed the title Revert #954 and update redis-py to at least 3.2.0 ISSUE-1006: Revert #954 and update redis-py to at least 3.2.0 Feb 21, 2019
@codecov
Copy link

codecov bot commented Feb 21, 2019

Codecov Report

Merging #1011 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1011      +/-   ##
==========================================
+ Coverage   88.61%   88.68%   +0.07%     
==========================================
  Files          63       63              
  Lines        6576     6569       -7     
  Branches      788      787       -1     
==========================================
- Hits         5827     5826       -1     
+ Misses        666      661       -5     
+ Partials       83       82       -1
Impacted Files Coverage Δ
kombu/transport/redis.py 90.88% <100%> (+0.78%) ⬆️

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 30f52ad...706f9f0. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Feb 21, 2019

Codecov Report

Merging #1011 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1011      +/-   ##
==========================================
+ Coverage   88.61%   88.68%   +0.07%     
==========================================
  Files          63       63              
  Lines        6576     6569       -7     
  Branches      788      787       -1     
==========================================
- Hits         5827     5826       -1     
+ Misses        666      661       -5     
+ Partials       83       82       -1
Impacted Files Coverage Δ
kombu/transport/redis.py 90.88% <100%> (+0.78%) ⬆️

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 30f52ad...706f9f0. Read the comment docs.

@lithammer
Copy link
Contributor Author

Personally I favour this solution over #1012 because it's cleaner. But realize that bumping the minimum required version of redis-py can be problematic for any number of reasons.

Copy link
Contributor

@georgepsarakis georgepsarakis left a comment

Choose a reason for hiding this comment

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

Nice work!

@auvipy auvipy merged commit eb6e4c8 into celery:master Feb 22, 2019
@lithammer lithammer deleted the issue-1006-solution-1 branch February 22, 2019 06:09
@thedrow
Copy link
Member

thedrow commented Feb 22, 2019

I guess we have to bump the minimum version.
This requires a new minor Kombu release. I'm not too happy about this but we gotta do what we can for the health of the project.

Thanks a lot for the contributions.

@lithammer
Copy link
Contributor Author

lithammer commented Feb 22, 2019

I mean there’s always the #1012 approach, but it adds technical debt to Kombu instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kombu 4.3.0: Unrecoverable error: ValueError('file descriptor cannot be a negative integer (-1)
4 participants