Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1769
@tequalsme Thanks for putting this together. The commit passes the full
build, and manually running it with an S3 processor worked great.
But I would prefer not to add `runner.setValidateExpressionUsage(false)` to
the unit tests for DyanmoDB, Kinesis, etc. They might use EL validation in the
future, and I don't think it should be necessary based on precedents like
ACCESS_KEY or PROXY_HOST that also use EL.
The problem appears to be a mismatch between those processors not
configured to use the ENDPOINT_OVERRIDE property descriptor, but are
nonetheless evaluating it when they call
`AbstractAWSProcessor::initializeRegionAndEndpoint`. That situation predates
your change, but adding EL causes the exception for those tests.
Instead, what would you think about the treatment given to REGION in that
same method ([AbstractAWSProcessor.java, ~line
210](https://github.com/tequalsme/nifi/blob/c56e026f0677a3894063d7d044cef38b63cc0066/nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/AbstractAWSProcessor.java#L210))?
protected void initializeRegionAndEndpoint(ProcessContext context) {
// if the processor supports REGION, get the configured region.
if (getSupportedPropertyDescriptors().contains(REGION)) {
final String region = context.getProperty(REGION).getValue();
if (region != null) {
this.region = Region.getRegion(Regions.fromName(region));
client.setRegion(this.region);
} else {
this.region = null;
}
}
// if the endpoint override has been configured, set the endpoint.
// (per Amazon docs this should only be configured at client
creation)
final String urlstr =
StringUtils.trimToEmpty(context.getProperty(ENDPOINT_OVERRIDE).evaluateAttributeExpressions().getValue());
if (!urlstr.isEmpty()) {
this.client.setEndpoint(urlstr);
}
}
The client is only modified when REGION is explicitly included on the
concrete processor class. We could add a second `if
(getSupportedPropertyDescriptors().contains(ENDPOINT_OVERRIDE)) {...`
protecting the expression language from being evaluated for ENDPOINT_OVERRIDE.
There is a similar, but different, approach used for PROXY_HOST on line 193 `if
(context.getProperty(PROXY_HOST).isSet()) {...`.
Now that I've made it this far, I'm not sure why there isn't such an if
statement even without the expression language, like REGION. Maybe I'm missing
something? Is this already familiar to you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
