Conversation
|
Warning Rate limit exceeded@githubzilla has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Proc as Process Startup
participant RS as RedisService
participant CFG as Flags / Local Config
participant Parser as txlog::ParseS3Url
participant LOG as Logger
Proc->>RS: InitTxLogService()
RS->>CFG: Read txlog_rocksdb_cloud_s3_url flag
alt flag set
RS->>Parser: Parse(s3_url_)
Parser-->>RS: bucket, object_path, endpoint_url, region
RS->>RS: Set txlog_rocksdb_cloud_config.s3_url_ and override fields
RS->>LOG: Info: using URL-based TxLog config
else flag empty
RS->>CFG: Read legacy txlog_rocksdb_cloud_config fields
RS->>RS: Use legacy cloud config values
end
RS-->>Proc: Continue initialization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/redis_service.cpp (1)
2451-2451: Comment formatting nitEnd-for comment spacing change only; no behavior impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/redis_service.cpp(4 hunks)store_handler(1 hunks)
🔇 Additional comments (2)
src/redis_service.cpp (2)
300-307: New S3 URL flag looks goodFlag name, default, help text, and placement under LOG_STATE_TYPE_RKDB_CLOUD are consistent with existing options.
1753-1760: LGTM on sourcing the URLReads from flag with config fallback in "local" section, consistent with other txlog cloud options.
githubzilla
force-pushed
the
add_s3_url_config
branch
from
db12796 to
844c706
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/redis_service.cpp (1)
1709-1716: Parse only when non-empty; ensure precedence; never log the raw URLNice addition. Please ensure downstream code only parses/uses this when non-empty and that precedence over legacy fields is actually enforced, and that no logs print s3_url_ directly. This echoes an earlier review point.
Run to verify guards and logging:
#!/bin/bash set -euo pipefail # Where is the URL parsed? rg -n -C3 -g '!**/build/**' -P '\bParseS3Url\s*\(' # Where is s3_url_ used and potentially logged? rg -n -C3 -g '!**/build/**' -P 's3_url_' rg -n -C2 -g '!**/build/**' -P 'LOG\\(.*s3_url_'
🧹 Nitpick comments (1)
src/redis_service.cpp (1)
300-307: Scope S3 URL flag to S3 builds; clarify help to discourage credentialsExpose this flag only when S3 is enabled to avoid confusing GCS-only builds, and make the help text explicit about precedence and not embedding credentials in the URL.
#if defined(LOG_STATE_TYPE_RKDB_CLOUD) ... -DEFINE_string(txlog_rocksdb_cloud_s3_url, +#if defined(LOG_STATE_TYPE_RKDB_S3) +DEFINE_string(txlog_rocksdb_cloud_s3_url, "", - "TxLog RocksDB cloud S3 URL. Format: s3://{bucket}/{path} or " - "http(s)://{host}:{port}/{bucket}/{path}. " - "Examples: s3://my-bucket/my-path, " - "http://localhost:9000/my-bucket/my-path. " - "This option takes precedence over legacy configuration options " - "if both are provided"); + "TxLog RocksDB cloud S3 URL. Format: s3://{bucket}/{path} or " + "http(s)://{host}:{port}/{bucket}/{path}. Examples: " + "s3://my-bucket/my-path, http://localhost:9000/my-bucket/my-path. " + "Do NOT include credentials in the URL; prefer FLAGS_aws_* or env vars. " + "When non-empty, this overrides endpoint/bucket/bucket_prefix/object_path/region.") +#endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/redis_service.cpp(4 hunks)store_handler(1 hunks)tx_service(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tx_service
🚧 Files skipped from review as they are similar to previous changes (1)
- store_handler
🔇 Additional comments (1)
src/redis_service.cpp (1)
2363-2363: LGTM (formatting-only).
1 participant
Summary by CodeRabbit
New Features
Chores
Style