You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "koushiro (via GitHub)" <gi...@apache.org> on 2025/12/23 07:12:42 UTC

[PR] refactor: Split redis service to new crate [opendal]

koushiro opened a new pull request, #7091:
URL: https://github.com/apache/opendal/pull/7091

   # Which issue does this PR close?
   
   Part of #6829 
   Closes #.
   
   # Rationale for this change
   
   <!--
   Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
   Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
   -->
   
   # What changes are included in this PR?
   
   - split redis service into a separate crate
   
   # Are there any user-facing changes?
   
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!---
   If there are any breaking changes to public APIs, please add the `breaking-changes` label.
   -->
   
   # AI Usage Statement
   
   Zed with GPT-5.1 Codex-Max
   


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] refactor: Split redis service to new crate [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #7091:
URL: https://github.com/apache/opendal/pull/7091


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] refactor: Split redis service to new crate [opendal]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7091:
URL: https://github.com/apache/opendal/pull/7091#discussion_r2642230359


##########
core/Cargo.toml:
##########
@@ -160,8 +160,8 @@ services-pcloud = ["dep:opendal-service-pcloud"]
 services-persy = ["dep:opendal-service-persy"]
 services-postgresql = ["dep:opendal-service-postgresql"]
 services-redb = ["dep:opendal-service-redb"]
-services-redis = ["opendal-core/services-redis"]
-services-redis-native-tls = ["opendal-core/services-redis-native-tls"]
+services-redis = ["opendal-service-redis?/rustls"]
+services-redis-native-tls = ["opendal-service-redis?/native-tls"]

Review Comment:
   The feature flags for `services-redis` and `services-redis-native-tls` are incorrect. Using `opendal-service-redis?/rustls` only tries to enable the `rustls` feature IF the dependency is already enabled, but doesn't actually enable the dependency itself. This means these features won't work as intended.
   
   The correct implementation should be:
   - `services-redis = ["dep:opendal-service-redis", "opendal-service-redis?/rustls"]`
   - `services-redis-native-tls = ["dep:opendal-service-redis", "opendal-service-redis?/native-tls"]`
   
   This ensures the dependency is enabled first with `dep:`, and then the appropriate TLS feature is selected.
   ```suggestion
   services-redis = ["dep:opendal-service-redis", "opendal-service-redis?/rustls"]
   services-redis-native-tls = ["dep:opendal-service-redis", "opendal-service-redis?/native-tls"]
   ```



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org