You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "G-XD (via GitHub)" <gi...@apache.org> on 2023/08/08 06:32:56 UTC
[GitHub] [incubator-opendal] G-XD opened a new pull request, #2812: Fix(services/s3): remove default region `us-east-1` for non-aws s3
G-XD opened a new pull request, #2812:
URL: https://github.com/apache/incubator-opendal/pull/2812
#2798
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2812: Fix(services/s3): remove default region `us-east-1` for non-aws s3
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2812:
URL: https://github.com/apache/incubator-opendal/pull/2812#discussion_r1286727795
##########
core/src/services/s3/backend.rs:
##########
@@ -772,19 +772,9 @@ impl Builder for S3Builder {
cfg.region = Some(v);
}
if cfg.region.is_none() {
- // AWS S3 requires region to be set.
- if self.endpoint.is_none()
- || self.endpoint.as_deref() == Some("https://s3.amazonaws.com")
- {
- return Err(Error::new(ErrorKind::ConfigInvalid, "region is missing")
- .with_operation("Builder::build")
- .with_context("service", Scheme::S3));
- }
-
- // For other compatible services, if we don't know region
- // after loading from builder and env, we can use `us-east-1`
- // as default.
- cfg.region = Some("us-east-1".to_string());
+ return Err(Error::new(ErrorKind::ConfigInvalid, "region is missing")
Review Comment:
To encourage users to programmatically find the region, we can include a helpful message suggesting the use of `S3::detect_region`.
For example: `region is missing. Please find it by S3::detect_region() or set them in env.`
--
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
[GitHub] [incubator-opendal] Xuanwo merged pull request #2812: fix(services/s3): remove default region `us-east-1` for non-aws s3
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #2812:
URL: https://github.com/apache/incubator-opendal/pull/2812
--
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