feat(config): refine defaults and improve region handling#1990
Merged
Conversation
- set default region to `us-east-1` - tighten and clean up related initialization paths
Contributor
There was a problem hiding this comment.
Pull request overview
Refines RustFS configuration defaults by switching the default region to us-east-1, and cleans up several initialization paths (notifications ARN parsing, KMS setup, and FTP/FTPS address normalization).
Changes:
- Set the default
RUSTFS_REGIONconstant tous-east-1and align config parsing to use it as the fallback region. - Refactor notification ARN parsing into a shared helper (
arn_to_target_id) and adjust notification init region fallback. - Refactor KMS initialization into smaller helpers and introduce a shared FTP/FTPS address normalization helper.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rustfs/src/init.rs | Adds helpers for ARN parsing, KMS config/build/start, and FTP/FTPS address normalization; adjusts region fallback behavior. |
| rustfs/src/config/mod.rs | Updates region defaulting comment and ensures region falls back to RUSTFS_REGION. |
| crates/config/src/constants/app.rs | Changes RUSTFS_REGION default from rustfs-global-0 to us-east-1. |
Comments suppressed due to low confidence (1)
rustfs/src/init.rs:273
configure_and_start_kms()already logs a success message, but theOk(())arm logs the same message again, resulting in duplicate info logs. Also, theErrarm says "Failed to configure" even though the helper may fail while starting as well; consider removing the extrainfo!and adjusting the warning to reflect both configure/start (or just log the error as-is).
match configure_and_start_kms(&service_manager, persisted_config, "persisted configuration").await {
Ok(()) => {
info!("KMS service configured and started successfully from persisted configuration");
}
Err(e) => {
warn!("Failed to configure KMS with persisted configuration: {}", e);
}
Comment on lines
349
to
352
| fn spawn_server<S>(server: S, protocol_name: &'static str) -> tokio::sync::broadcast::Sender<()> | ||
| where | ||
| S: Future<Output = Result<(), Box<dyn std::error::Error>>> + Send + 'static, | ||
| { |
There was a problem hiding this comment.
spawn_server uses the Future trait in its bound (S: Future<...>) but Future is not imported or fully qualified in this module, which will fail to compile. Either import std::future::Future (or qualify it) or remove this unused helper and keep the inlined spawn logic in the FTP/FTPS initializers.
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: houseme <housemecn@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
us-east-1Type of Change
Related Issues
Summary of Changes
Checklist
make pre-commitImpact
Additional Notes
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.