Lehel44 commented on code in PR #7051:
URL: https://github.com/apache/nifi/pull/7051#discussion_r1142586898
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -150,6 +160,16 @@ public abstract class AbstractS3Processor extends
AbstractAWSCredentialsProvider
.dependsOn(SIGNER_OVERRIDE, CUSTOM_SIGNER)
.build();
+ public static final String S3_REGION_ATTRIBUTE = "s3.region" ;
+ static final AllowableValue ATTRIBUTE_DRIVEN_REGION = new
AllowableValue("Use '" + S3_REGION_ATTRIBUTE + "' Attribute",
Review Comment:
What do you think of ATTRIBUTE_DEFINED_REGION? For the value the standard
seems to be lowercase with dashes, so it would be
"attribute-driven/defined-region". The description could be a bit simpler like
"Uses the S3 Region Attribute as region"
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -198,6 +218,88 @@ protected AmazonS3Client createClient(final ProcessContext
context, final AWSCre
return s3;
}
+ protected Region resolveRegion(final ProcessContext context, final
Map<String, String> attributes) {
+ String regionValue = context.getProperty(S3_CUSTOM_REGION).getValue();
+
+ if (ATTRIBUTE_DRIVEN_REGION.getValue().equals(regionValue)) {
+ regionValue = attributes.get(S3_REGION_ATTRIBUTE);
+ }
+
+ return parseRegionValue(regionValue);
+ }
+
+ private Region parseRegionValue(String regionValue) {
+ if (regionValue == null) {
+ throw new ProcessException(format("[%s] was selected as region
source but [%s] attribute does not exist", ATTRIBUTE_DRIVEN_REGION,
S3_REGION_ATTRIBUTE));
+ }
+
+ try {
+ return Region.getRegion(Regions.fromName(regionValue));
+ } catch (Exception e) {
+ throw new ProcessException(format("The [%s] attribute contains an
invalid region value [%s]", S3_REGION_ATTRIBUTE, regionValue), e);
+ }
+ }
+
+ protected void setClientBasedOnRegionAttribute(final ProcessContext
context, final Map<String, String> attributes) {
+ String regionValue = context.getProperty(S3_CUSTOM_REGION).getValue();
+
+ if (ATTRIBUTE_DRIVEN_REGION.getValue().equals(regionValue)) {
+ regionValue = attributes.get(S3_REGION_ATTRIBUTE);
+ Region region = parseRegionValue(regionValue);
Review Comment:
I think resolveRegion should be called here.
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -150,6 +160,16 @@ public abstract class AbstractS3Processor extends
AbstractAWSCredentialsProvider
.dependsOn(SIGNER_OVERRIDE, CUSTOM_SIGNER)
.build();
+ public static final String S3_REGION_ATTRIBUTE = "s3.region" ;
+ static final AllowableValue ATTRIBUTE_DRIVEN_REGION = new
AllowableValue("Use '" + S3_REGION_ATTRIBUTE + "' Attribute",
+ "Use '" + S3_REGION_ATTRIBUTE + "' Attribute",
+ "Inspects the '" + S3_REGION_ATTRIBUTE + "' FlowFile attribute to
determine the used S3 region.");
+
+ public static final PropertyDescriptor S3_CUSTOM_REGION = new
PropertyDescriptor.Builder()
Review Comment:
Should we call this 'custom' region? There is one option to define a custom
region but overall it includes the regular regions from aws.
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/AbstractAWSProcessor.java:
##########
@@ -312,21 +312,9 @@ public void onTrigger(final ProcessContext context, final
ProcessSessionFactory
*/
public abstract void onTrigger(final ProcessContext context, final
ProcessSession session) throws ProcessException;
- protected Region getRegionAndInitializeEndpoint(final ProcessContext
context, final AmazonWebServiceClient client) {
- final Region region;
- // if the processor supports REGION, get the configured region.
- if (getSupportedPropertyDescriptors().contains(REGION)) {
- final String regionValue = context.getProperty(REGION).getValue();
- if (regionValue != null) {
- region = Region.getRegion(Regions.fromName(regionValue));
- if (client != null) {
- client.setRegion(region);
- }
- } else {
- region = null;
- }
- } else {
- region = null;
+ protected void initializeEndpoint(final Region region, final
ProcessContext context, final AmazonWebServiceClient client) {
Review Comment:
I think the Region parameter is not needed, you can get it in the method by
calling getRegion(context).
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
