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

Support authenticating using Redis usernames #1273

Merged
merged 2 commits into from
Jun 10, 2023
Merged

Conversation

espadrine
Copy link
Contributor

Redis added the AUTH command with only a password at first, and that is what the current Flower code assumes.
Redis added usernames in the AUTH command later, and added support for them in the redis-py library.

In this patch, we add support for usernames in Flower.

Note that while the latest release of Kombu allows many versions of the redis-py package, to support usernames, you need a version that includes the commit linked above. We tested redis-py v4.5.4.

@mher
Copy link
Owner

mher commented May 13, 2023

This change is not backwards compatible

@espadrine
Copy link
Contributor Author

Thanks for discussing it!

What backwards incompatibility does it trigger?

Redis added the AUTH command with only a password at first, and that is
what the current Flower code assumes.
Redis added usernames in the AUTH command later, and added support for
them in the redis-py library here:
redis/redis-py@8df8cd5

In this patch, we add support for usernames in Flower.
@espadrine
Copy link
Contributor Author

I added tests, in case what you were hinting at was that we want to detect backwards incompatibility on it.

I also tested the following:

  1. Launched docker compose up;
  2. Ran docker compose run flower celery -A tasks call tasks.add --args '[1, 1]';
  3. Checked on http://localhost:5555/ that the task was successful.

The default docker-compose.yml configuration does not use a Redis username, which makes me believe there is no backwards incompatibility, but of course there could be something I overlooked.

(As an aside, tests/call-tasks.sh has commands with outdated positional parameters which are rejected by celery on the project Dockerfile. I can change that if you wish.)

@mher mher merged commit 72abc9e into mher:master Jun 10, 2023
5 checks passed
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.

None yet

2 participants