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

Add support in the redis result backend for authenticating with a username #6750

Merged
merged 2 commits into from
May 2, 2021

Conversation

thedrow
Copy link
Member

@thedrow thedrow commented Apr 29, 2021

Note: Before submitting this pull request, please review our contributing
guidelines
.

Description

Previously, the username was ignored from the URI.
Starting from Redis>=6.0, that shouldn't be the case since ACL support has landed.

Fixes #6422.

…rname.

Previously, the username was ignored from the URI.
Starting from Redis>=6.0, that shouldn't be the case since ACL support has landed.

Fixes #6422.
@codecov
Copy link

codecov bot commented Apr 29, 2021

Codecov Report

Merging #6750 (398eaad) into master (0953a4d) will increase coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6750      +/-   ##
==========================================
+ Coverage   70.50%   70.69%   +0.18%     
==========================================
  Files         138      138              
  Lines       16485    16559      +74     
  Branches     2065     2079      +14     
==========================================
+ Hits        11623    11706      +83     
+ Misses       4660     4658       -2     
+ Partials      202      195       -7     
Flag Coverage Δ
unittests 70.69% <100.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
celery/app/defaults.py 97.33% <ø> (ø)
celery/backends/redis.py 82.95% <100.00%> (+0.14%) ⬆️
celery/worker/request.py 96.91% <0.00%> (-0.03%) ⬇️
celery/app/amqp.py 94.09% <0.00%> (ø)
celery/app/base.py 58.93% <0.00%> (ø)
celery/bin/amqp.py 0.00% <0.00%> (ø)
celery/app/utils.py 47.61% <0.00%> (ø)
celery/bin/control.py 0.00% <0.00%> (ø)
celery/app/task.py 93.83% <0.00%> (+0.01%) ⬆️
... and 6 more

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 0953a4d...398eaad. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Apr 29, 2021

This pull request introduces 1 alert and fixes 2 when merging b93fe34 into b0326ab - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Module is imported with 'import' and 'import from'

auvipy
auvipy previously approved these changes Apr 29, 2021
docs/userguide/configuration.rst Show resolved Hide resolved
@thedrow thedrow merged commit ae20f2f into master May 2, 2021
@thedrow
Copy link
Member Author

thedrow commented May 2, 2021

@auvipy I addressed your comment.
I'm merging this per your previous review as I want to expedite releasing beta 2.

@thedrow thedrow deleted the redis-username branch May 2, 2021 08:34
@lgtm-com
Copy link

lgtm-com bot commented May 2, 2021

This pull request introduces 1 alert and fixes 2 when merging 398eaad into b0326ab - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Module is imported with 'import' and 'import from'

@auvipy
Copy link
Member

auvipy commented May 3, 2021

sure.

jeyrce pushed a commit to jeyrce/celery that referenced this pull request Aug 25, 2021
…rname (celery#6750)

* Add support in the redis result backend for authenticating with a username.

Previously, the username was ignored from the URI.
Starting from Redis>=6.0, that shouldn't be the case since ACL support has landed.

Fixes celery#6422.

* Mention which version added support for this setting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Celery cannot connect to redis >6.0 using ACL user:password broker url
2 participants