From issues-return-97795-archive-asf-public=cust-asf.ponee.io@nifi.apache.org Wed May 27 03:52:19 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 89FDF180608 for ; Wed, 27 May 2020 05:52:19 +0200 (CEST) Received: (qmail 10829 invoked by uid 500); 27 May 2020 03:52:18 -0000 Mailing-List: contact issues-help@nifi.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@nifi.apache.org Delivered-To: mailing list issues@nifi.apache.org Received: (qmail 10818 invoked by uid 99); 27 May 2020 03:52:18 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 27 May 2020 03:52:18 +0000 From: =?utf-8?q?GitBox?= To: issues@nifi.apache.org Subject: =?utf-8?q?=5BGitHub=5D_=5Bnifi=5D_sjyang18_commented_on_a_change_in_pull_req?= =?utf-8?q?uest_=234265=3A_NIFI-7434=3A_Endpoint_suffix_property_in_AzureSto?= =?utf-8?q?rageAccount_NIFI_processors?= Message-ID: <159055153831.30832.14765365887178271063.asfpy@gitbox.apache.org> Date: Wed, 27 May 2020 03:52:18 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit In-Reply-To: References: 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: users@infra.apache.org