Support callable passwords for credential refresh on reconnect#2476
Support callable passwords for credential refresh on reconnect#2476auvipy merged 12 commits intocelery:mainfrom
Conversation
Resolves celery#2206. Allow the `password` parameter to be a callable (function, class with `__call__`, etc.) that is invoked fresh on every connection/reconnection attempt. This enables credential refresh for expiring tokens (OAuth, JWT, other tokens) without requiring custom transport subclasses. The callable is resolved to a string in `establish_connection()` before passing to py-amqp, so downstream code is unaffected. String passwords continue to work identically (backward compatible). The [broker_failover_strategy](https://docs.celeryq.dev/en/main/userguide/configuration.html#broker-failover-strategy) was considered, however a fresh token is only obtained after the stale one has already failed to authenticate. As well as the unit tests included in this commit, I've also verified with a local project that it works in the manner intended and can be used to easily fetch a fresh token on reconnect.
There was a problem hiding this comment.
Pull request overview
Adds support for providing a callable password so a fresh credential can be fetched on each connect/reconnect attempt (targeted at AMQP via pyamqp), with accompanying documentation and unit tests.
Changes:
- Resolve callable passwords inside
pyamqp.Transport.establish_connection()before creating the underlyingamqp.Connection. - Document callable-password behavior on
kombu.Connectionand updateConnection.as_uri()to handle callable passwords. - Add unit tests covering callable passwords for initial connect and reconnect behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
kombu/transport/pyamqp.py |
Resolves callable passwords at connect-time and passes the resolved string to amqp.Connection. |
kombu/connection.py |
Documents callable passwords and updates as_uri() to handle callables. |
t/unit/transport/test_pyamqp.py |
Adds tests verifying callable password resolution and per-reconnect invocation for the pyamqp transport. |
t/unit/test_connection.py |
Adds tests ensuring callable passwords are preserved on the Connection object and don’t crash as_uri(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Avoids invoking a potentially expensive callable (e.g. network token refresh) when the result will be masked anyway.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2476 +/- ##
==========================================
+ Coverage 82.31% 82.32% +0.01%
==========================================
Files 79 79
Lines 10097 10104 +7
Branches 1154 1157 +3
==========================================
+ Hits 8311 8318 +7
Misses 1584 1584
Partials 202 202 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
flamble
force-pushed
the
feature/callable-password-support
branch
from
15b5820 to
979ddcf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Clarify comments as per copilots suggestion around callable password usage - Address @auvipy's comment to use versionadded for 5.7
|
@auvipy - please let me know if you need anything else from me on this. Thanks! |
auvipy
left a comment
auvipy
left a comment
There was a problem hiding this comment.
yes, please fix the merge conflicts
|
@auvipy — thanks again for your time on this so far! Just checking if there’s anything else you’d like me to address, or if it’s ready for another look when you have a moment. Appreciate your help. |
|
relevant celery/celery#9454 |
|
Thanks @auvipy ! |
Resolves #2206.
Allow the
passwordparameter to be a callable (function, class with__call__, etc.) that is invoked fresh on every connection/reconnection attempt. This enables credential refresh for expiring tokens (OAuth, JWT, other tokens) without requiring custom transport subclasses.The callable is resolved to a string in
establish_connection()before passing to py-amqp, so downstream code is unaffected. String passwords continue to work identically (backward compatible).The broker_failover_strategy was considered, however a fresh token is only obtained after the stale one has already failed to authenticate.
As well as the unit tests included in this commit, I've also verified with a local project that it works in the manner intended and can be used to easily fetch a fresh token on reconnect.