sjyang18 commented on a change in pull request #4265:
URL: https://github.com/apache/nifi/pull/4265#discussion_r430548677
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/services/azure/storage/TestAzureStorageCredentialsControllerServiceLookup.java
##########
@@ -71,28 +73,32 @@ public void testLookupServiceA() {
final AzureStorageCredentialsDetails storageCredentialsDetails =
lookupService.getStorageCredentialsDetails(attributes);
assertNotNull(storageCredentialsDetails);
assertEquals("Account_A",
storageCredentialsDetails.getStorageAccountName());
+ assertEquals("accountsuffix.core.windows.net",
storageCredentialsDetails.getStorageSuffix());
}
@Test
public void testLookupServiceB() {
Review comment:
null default is handled in SDK. So, the current behavior does not get
affected.
private static String getDNS(String service, String base) {
if (base == null) {
base = DEFAULT_DNS;
}
return String.format(DNS_NAME_FORMAT, service, base);
}
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java
##########
@@ -85,6 +86,20 @@
.sensitive(true)
.build();
+ public static final PropertyDescriptor ENDPOINT_SUFFIX = new
PropertyDescriptor.Builder()
+ .name("storage-endpoint-suffix")
+ .displayName("Common Storage Account Endpoint Suffix")
+ .description(
+ "Storage accounts in public Azure always use a common FQDN
suffix. " +
+ "Override this endpoint suffix with a different suffix in
certain circumsances (like Azure Stack or non-public Azure regions). " +
+ "The preferred way is to configure them through a
controller service specified in the Storage Credentials property. " +
+ "The controller service can provide a common/shared
configuration for multiple/all Azure processors. Furthermore, the credentials "
+
+ "can also be looked up dynamically with the 'Lookup'
version of the service.")
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .required(false)
Review comment:
Null default value is acceptable. The Azure Storage SDK sets the default
end suffix, if we pass null. Do we still need a validator in this case? I
validated the regression behavior without setting end suffix property.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java
##########
@@ -85,6 +86,20 @@
.sensitive(true)
.build();
+ public static final PropertyDescriptor ENDPOINT_SUFFIX = new
PropertyDescriptor.Builder()
+ .name("storage-endpoint-suffix")
+ .displayName("Common Storage Account Endpoint Suffix")
+ .description(
+ "Storage accounts in public Azure always use a common FQDN
suffix. " +
+ "Override this endpoint suffix with a different suffix in
certain circumsances (like Azure Stack or non-public Azure regions). " +
+ "The preferred way is to configure them through a
controller service specified in the Storage Credentials property. " +
+ "The controller service can provide a common/shared
configuration for multiple/all Azure processors. Furthermore, the credentials "
+
+ "can also be looked up dynamically with the 'Lookup'
version of the service.")
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .required(false)
Review comment:
In order to support Azure Stack on-premise, this should be an editable
free form.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java
##########
@@ -85,6 +86,20 @@
.sensitive(true)
.build();
+ public static final PropertyDescriptor ENDPOINT_SUFFIX = new
PropertyDescriptor.Builder()
+ .name("storage-endpoint-suffix")
+ .displayName("Common Storage Account Endpoint Suffix")
+ .description(
+ "Storage accounts in public Azure always use a common FQDN
suffix. " +
+ "Override this endpoint suffix with a different suffix in
certain circumsances (like Azure Stack or non-public Azure regions). " +
+ "The preferred way is to configure them through a
controller service specified in the Storage Credentials property. " +
+ "The controller service can provide a common/shared
configuration for multiple/all Azure processors. Furthermore, the credentials "
+
+ "can also be looked up dynamically with the 'Lookup'
version of the service.")
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
Review comment:
I took the option and set StandardValidators.NON_BLANK_VALIDATOR to
EndpointSuffix property.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-services-api/src/main/java/org/apache/nifi/services/azure/storage/AzureStorageCredentialsDetails.java
##########
@@ -22,17 +22,24 @@
private final String storageAccountName;
+ private final String storageSuffix;
+
private final StorageCredentials storageCredentials;
- public AzureStorageCredentialsDetails(String storageAccountName,
StorageCredentials storageCredentials) {
+ public AzureStorageCredentialsDetails(String storageAccountName, String
storageSuffix, StorageCredentials storageCredentials) {
Review comment:
done. Thanks for the tip.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/processors/azure/storage/queue/GetAzureQueueStorageIT.java
##########
@@ -39,6 +39,7 @@ public void setUp() throws StorageException {
cloudQueue.addMessage(new CloudQueueMessage("Dummy Message 1"),
604800, 0, null, null);
cloudQueue.addMessage(new CloudQueueMessage("Dummy Message 2"),
604800, 0, null, null);
cloudQueue.addMessage(new CloudQueueMessage("Dummy Message 3"),
604800, 0, null, null);
+
Review comment:
Done. But, I am still updating this file with a test case.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java
##########
@@ -85,6 +86,20 @@
.sensitive(true)
.build();
+ public static final PropertyDescriptor ENDPOINT_SUFFIX = new
PropertyDescriptor.Builder()
+ .name("storage-endpoint-suffix")
+ .displayName("Common Storage Account Endpoint Suffix")
+ .description(
+ "Storage accounts in public Azure always use a common FQDN
suffix. " +
+ "Override this endpoint suffix with a different suffix in
certain circumsances (like Azure Stack or non-public Azure regions). " +
+ "The preferred way is to configure them through a
controller service specified in the Storage Credentials property. " +
+ "The controller service can provide a common/shared
configuration for multiple/all Azure processors. Furthermore, the credentials "
+
+ "can also be looked up dynamically with the 'Lookup'
version of the service.")
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .required(false)
Review comment:
I added NON_BLANK_VALIDATOR.
----------------------------------------------------------------
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:
[email protected]