Copilot commented on code in PR #18268:
URL:
https://github.com/apache/dolphinscheduler/pull/18268#discussion_r3296514511
##########
dolphinscheduler-authentication/dolphinscheduler-aws-authentication/src/main/java/org/apache/dolphinscheduler/authentication/aws/AmazonS3ClientFactory.java:
##########
@@ -26,23 +26,28 @@
import com.amazonaws.regions.Regions;
import com.amazonaws.services.s3.AmazonS3;
import com.amazonaws.services.s3.AmazonS3ClientBuilder;
+import com.amazonaws.util.StringUtils;
@UtilityClass
public class AmazonS3ClientFactory {
public AmazonS3 createAmazonS3Client(Map<String, String> awsProperties) {
AWSCredentialsProvider awsCredentialsProvider =
AWSCredentialsProviderFactor.credentialsProvider(awsProperties);
- Regions regions =
Regions.fromName(awsProperties.get(AwsConfigurationKeys.AWS_REGION));
+ String region = awsProperties.get(AwsConfigurationKeys.AWS_REGION);
String endpoint = awsProperties.get(AwsConfigurationKeys.AWS_ENDPOINT);
if (endpoint != null && !endpoint.isEmpty()) {
+ if (StringUtils.isNullOrEmpty(region)) {
+ throw new IllegalArgumentException("The aws.s3.region cannot
be empty");
+ }
Review Comment:
`AmazonS3ClientFactory` introduces `com.amazonaws.util.StringUtils`, which
is easy to confuse with the widely used `org.apache.commons.lang3.StringUtils`
in this repo and is only used for a simple null/empty check. Consider avoiding
the AWS `StringUtils` import and using the same null/empty style as the
existing `endpoint` check (or the repo-standard Apache/Guava string helpers) to
keep this factory consistent and reduce ambiguity.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
