korbit-ai[bot] commented on code in PR #32372:
URL: https://github.com/apache/superset/pull/32372#discussion_r1969217848
##########
superset/async_events/cache_backend.py:
##########
@@ -95,6 +95,10 @@ def from_config(cls, config: Dict[str, Any]) ->
"RedisCacheBackend":
"ssl_cert_reqs": config.get("CACHE_REDIS_SSL_CERT_REQS",
"required"),
"ssl_ca_certs": config.get("CACHE_REDIS_SSL_CA_CERTS", None),
}
+
+ if configured_username := config.get("CACHE_REDIS_USER", ""):
+ kwargs["username"] = configured_username
Review Comment:
### Missing Context for Redis Username Configuration <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The newly added username configuration option lacks any explanation about
its purpose or why it's being added separately from other Redis connection
parameters.
###### Why this matters
Without context, future maintainers won't understand why username is handled
differently from other Redis connection parameters or when they should use it.
###### Suggested change ∙ *Feature Preview*
Add an inline comment explaining the conditional handling: `# Handle
username separately as it's optional in modern Redis auth`
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/92babf76-3923-434b-a60a-3e5248175413?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:3422ab0c-9a20-490e-b2f2-0f8f72794ea9 -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
