turcsanyip commented on a change in pull request #5136:
URL: https://github.com/apache/nifi/pull/5136#discussion_r647320815
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/eventhub/ConsumeAzureEventHub.java
##########
@@ -91,7 +91,8 @@
})
public class ConsumeAzureEventHub extends AbstractSessionFactoryProcessor {
- private static final String FORMAT_STORAGE_CONNECTION_STRING =
"DefaultEndpointsProtocol=https;AccountName=%s;AccountKey=%s";
+ private static final String FORMAT_STORAGE_CONNECTION_STRING_FOR_KEY =
"DefaultEndpointsProtocol=https;AccountName=%s;AccountKey=%s";
+ private static final String FORMAT_STORAGE_CONNECTION_STRING_FOR_TOKEN =
"BlobEndpoint=https://%s.blob.core.windows.net/;SharedAccessSignature=%s";;
Review comment:
To make it more straightforward, I would suggest using `FOR_ACCOUNT_KEY`
and `FOR_SAS_TOKEN` suffixes.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/eventhub/ConsumeAzureEventHub.java
##########
@@ -332,6 +345,24 @@ public void setWriterFactory(RecordSetWriterFactory
writerFactory) {
.valid(false)
.build());
}
+
+ if (storageAccountKey == null && storageSasToken == null) {
+ results.add(new ValidationResult.Builder()
+ .subject("Storage Account Key or Storage SAS Token")
Review comment:
`getDisplayName()` could be used as for the explanation below.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/processors/azure/eventhub/TestConsumeAzureEventHub.java
##########
@@ -132,12 +135,42 @@ public void
testProcessorConfigValidityWithManagedIdentityFlag() throws Initiali
testRunner.assertValid();
}
+ @Test
+ public void testProcessorConfigValidityWithNeitherStorageKeyNorTokenSet() {
+ TestRunner testRunner = TestRunners.newTestRunner(processor);
+
testRunner.setProperty(ConsumeAzureEventHub.EVENT_HUB_NAME,eventHubName);
+ testRunner.assertNotValid();
+ testRunner.setProperty(ConsumeAzureEventHub.NAMESPACE,namespaceName);
+ testRunner.assertNotValid();
+ testRunner.setProperty(ConsumeAzureEventHub.ACCESS_POLICY_NAME,
policyName);
+ testRunner.setProperty(ConsumeAzureEventHub.POLICY_PRIMARY_KEY,
policyKey);
+ testRunner.assertNotValid();
+ testRunner.setProperty(ConsumeAzureEventHub.STORAGE_ACCOUNT_NAME,
storageAccountName);
+ testRunner.assertNotValid();
+ }
+
+ @Test
+ public void testProcessorConfigValidityWithBothStorageKeyAndTokenSet() {
+ TestRunner testRunner = TestRunners.newTestRunner(processor);
+
testRunner.setProperty(ConsumeAzureEventHub.EVENT_HUB_NAME,eventHubName);
+ testRunner.assertNotValid();
+ testRunner.setProperty(ConsumeAzureEventHub.NAMESPACE,namespaceName);
+ testRunner.assertNotValid();
+ testRunner.setProperty(ConsumeAzureEventHub.ACCESS_POLICY_NAME,
policyName);
+ testRunner.setProperty(ConsumeAzureEventHub.POLICY_PRIMARY_KEY,
policyKey);
+ testRunner.assertNotValid();
+ testRunner.setProperty(ConsumeAzureEventHub.STORAGE_ACCOUNT_NAME,
storageAccountName);
+ testRunner.setProperty(ConsumeAzureEventHub.STORAGE_ACCOUNT_KEY,
storageAccountKey);
+ testRunner.setProperty(ConsumeAzureEventHub.STORAGE_SAS_TOKEN,
storageSasToken);
+ testRunner.assertNotValid();
+ }
+
Review comment:
It would be good to have some tests for the valid cases as well. (when
Storage Account Key specified only, and when Storage SAS Token only).
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/processors/azure/eventhub/TestConsumeAzureEventHub.java
##########
@@ -150,7 +183,7 @@ public void testReceivedApplicationProperties() throws
Exception {
@Test
public void testReceiveOne() throws Exception {
- final Iterable<EventData> eventDataList =
Arrays.asList(EventData.create("one".getBytes(StandardCharsets.UTF_8)));
+ final Iterable<EventData> eventDataList =
Collections.singletonList(EventData.create("one"
.getBytes(StandardCharsets.UTF_8)));
Review comment:
Typo / wrong formatting: `"one".getBytes()`
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/eventhub/ConsumeAzureEventHub.java
##########
@@ -332,6 +345,24 @@ public void setWriterFactory(RecordSetWriterFactory
writerFactory) {
.valid(false)
.build());
}
+
+ if (storageAccountKey == null && storageSasToken == null) {
+ results.add(new ValidationResult.Builder()
+ .subject("Storage Account Key or Storage SAS Token")
+ .explanation(String.format("Either %s or %s should be
set.",
Review comment:
The explanation will be concatenated into the middle of the validation
failure message so it should start with lower case (_"...is invalid because
either..."_)
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
