Skip to content

Conversation

@mdalp
Copy link
Contributor

@mdalp mdalp commented Sep 19, 2025

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

Description

In order to simplify debugging and monitoring it would be great to be able to customise the client_name used by celery.
This PR goes hand in hand with celery/kombu#2367, and the feature will start working only once both are merged, before no client_name will be set, but the code shouldn't break.

Notes

I added the new parameter to the docs, I didn't know what to put as versionadded:: so I ended up picking a reasonable version, but I'm aware that this is just a placeholder at this stage.

@Nusnus Nusnus requested a review from Copilot September 19, 2025 15:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for configuring a Redis client name in Celery, which allows users to identify Redis connections in monitoring tools for easier debugging. The change introduces a new configuration parameter redis_client_name that gets passed to the Redis backend connection parameters.

Key Changes

  • Added new configuration option redis_client_name for customizing Redis client identification
  • Updated Redis backend to use the client name configuration when establishing connections
  • Added comprehensive test coverage for both set and unset client name scenarios

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
celery/backends/redis.py Adds client_name parameter to Redis connection configuration
docs/userguide/configuration.rst Documents the new redis_client_name configuration option
t/unit/backends/test_redis.py Adds unit tests for client name functionality

@codecov
Copy link

codecov bot commented Sep 19, 2025

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.67%. Comparing base (a5e9ce5) to head (d9c4d15).
⚠️ Report is 73 commits behind head on main.

Files with missing lines Patch % Lines
celery/utils/time.py 66.66% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9900      +/-   ##
==========================================
- Coverage   78.72%   78.67%   -0.06%     
==========================================
  Files         153      153              
  Lines       19280    19291      +11     
  Branches     2567     2568       +1     
==========================================
- Hits        15179    15177       -2     
- Misses       3801     3816      +15     
+ Partials      300      298       -2     
Flag Coverage Δ
unittests 78.65% <73.33%> (-0.06%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@auvipy auvipy self-requested a review September 20, 2025 04:00
@auvipy auvipy added this to the 5.6.0 milestone Sep 20, 2025
@auvipy auvipy force-pushed the support-redis-client-name branch from f5e2dff to 4a353f4 Compare September 20, 2025 12:36
@Nusnus Nusnus force-pushed the support-redis-client-name branch from 4a353f4 to f90fc16 Compare September 20, 2025 17:27
@auvipy auvipy force-pushed the support-redis-client-name branch from f90fc16 to 541e6f4 Compare September 23, 2025 11:22
…support-redis-client-name

# Conflicts:
#	docs/userguide/configuration.rst
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

is the cod coverage report right? can you please cross check and improve if needed?

@mdalp
Copy link
Contributor Author

mdalp commented Sep 25, 2025

Very odd, that's definitely unexpected, I'll have a look

@mdalp
Copy link
Contributor Author

mdalp commented Sep 25, 2025

OK, so the issue with codecov comes from merging main into this branch 8840502, specifically this #9862.

Current main looks fine, so I'll merge main back into this branch.

@mdalp
Copy link
Contributor Author

mdalp commented Sep 25, 2025

For some reason, fixing this commit history turned out more complicated than expected.
I'm confident that merging this PR won't cause issues with the code coverage, but if you feel strongly about it I can spend a bit more time trying to fix the git history, but that'll take a bit longer.

@mdalp mdalp force-pushed the support-redis-client-name branch from 79317ad to d9c4d15 Compare September 25, 2025 13:14
@auvipy
Copy link
Member

auvipy commented Sep 25, 2025

you do not need to fix git history. if possible add some more tests and relevant docs

@auvipy auvipy merged commit 8297e70 into celery:main Sep 28, 2025
400 of 405 checks passed
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.

2 participants