-
Notifications
You must be signed in to change notification settings - Fork 11
redis tls support #95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds TLS support for Redis (new rustls dependency, redis TLS features, server Rustls provider initialization) and updates twmq to use Redis Cluster hash tags for all queue-related keys; documentation and tests updated to show rediss:// examples and hash-tagged key patterns. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Cargo.toml`:
- Line 101: The Cargo.toml currently relies on redis = "0.31.0" with TLS
features but must also add an explicit rustls dependency; update Cargo.toml to
include a rustls crate (e.g., rustls or rustls-native-certs as appropriate) and
then ensure you install a rustls crypto provider at application startup (before
any TLS connections are created) by calling the provider installer (for example,
rustls::crypto::ring::default_provider().install_default()) in your main/startup
initialization routine so the redis TLS connections do not fail with "crypto
provider not installed".
In `@server/DOCKER.md`:
- Around line 135-136: The commented Redis-over-TLS line in the DOCKER.md
docker-compose example breaks YAML indentation; fix it by aligning the comment
with the other environment list items so it matches the same indentation level
as the other `-` entries (e.g., ensure the `# -
APP__REDIS__URL=rediss://redis:6379` comment is indented to the same column as
other `- APP__...` environment lines), so users can safely uncomment
`APP__REDIS__URL=rediss://redis:6379` without corrupting YAML structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
twmq/src/multilane.rs (1)
1045-1052:⚠️ Potential issue | 🟠 MajorReplace plain
queue_id()withredis_hash_tag()in trim script KEYS.Trim scripts at lines 1046 and 1193 pass untagged
KEYS[1]while all other KEYS useredis_hash_tag()internally. This breaks Redis cluster slot alignment, causing CROSSSLOT errors or silent pruning failures. All key-building methods (success_list_name(),job_data_hash_name(), etc.) already useredis_hash_tag()which wraps the queue_id with{}for proper hash tagging.Fix
- .key(self.queue_id()) + .key(self.redis_hash_tag())Apply at lines 1046 and 1193.
This reverts commit d729f5f.
Summary by CodeRabbit
New Features
Compatibility
Documentation
Chores