Add Redis client self-identification for Apache Airflow#61866
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
3c61e50 to
33cb2e3
Compare
33cb2e3 to
037bd29
Compare
|
Assuming tests pass. |
8d0d324 to
9a96aca
Compare
|
@vchomakov Converting to draft — this PR doesn't yet meet our Pull Request quality criteria.
See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is not a rejection — just an invitation to bring the PR up to standard. No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
bdbbe0f to
4a2246f
Compare
0e1dcb2 to
327177d
Compare
|
restarted the tests - lets see |
327177d to
f63c53f
Compare
Thanks @eladkal ! I addressed the test breakage. All tests pass now. Can you please allow the run of the remaining ones? |
There was a problem hiding this comment.
Pull request overview
Adds Redis client self-identification in the Redis provider so Redis servers can attribute connections to Apache Airflow via CLIENT INFO / CLIENT SETINFO.
Changes:
- Adds driver/client identification parameters when instantiating
redis.Redis(prefersDriverInfo, falls back tolib_name). - Updates unit tests to validate the extra kwargs passed to the Redis client constructor.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| providers/redis/src/airflow/providers/redis/hooks/redis.py | Adds Redis client self-identification via DriverInfo or lib_name fallback. |
| providers/redis/tests/unit/redis/hooks/test_redis.py | Adjusts assertions to account for additional Redis constructor kwargs related to identification. |
|
@vchomakov You replied to an earlier review round over a month ago, but several threads are still marked unresolved and the PR has not moved since. Converting to draft — this PR doesn't yet meet our Pull Request quality criteria.
See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is not a rejection — just an invitation to bring the PR up to standard. No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
|
Quick follow-up to the triage comment above — one clarification on the "Unresolved review comments" item: Once you believe a thread has been addressed — whether by pushing a fix, or by replying in-thread with an explanation of why the suggestion doesn't apply — please mark the thread as resolved yourself by clicking the "Resolve conversation" button at the bottom of each thread. Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as "still waiting on the author" and keeps the PR from moving forward. The author doing the resolve-click is the expected convention on this project. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
Thanks for the comment, @potiuk |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
Add Redis client self-identification for Apache Airflow's Redis provider, allowing Redis servers to identify Airflow connections via the
CLIENT INFOcommand.This implementation follows the Redis documentation recommendation for client libraries (https://redis.io/docs/latest/commands/client-setinfo/) and uses a dual approach for compatibility:
DriverInfoclass for redis-py 5.1.0+ to add "apache-airflow" as upstream driverlib_nameparameter for older redis-py versionsThis makes it easier for Redis administrators to monitor and debug connections from Airflow.
Changes:
RedisHook.get_conn()to include driver identificationWas generative AI tooling used to co-author this PR?
Generated-by: Augment Code following the guidelines