jfrazee commented on a change in pull request #5303:
URL: https://github.com/apache/nifi/pull/5303#discussion_r804876565
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/eventhub/utils/AzureEventHubUtils.java
##########
@@ -33,6 +34,10 @@
public final class AzureEventHubUtils {
public static final String MANAGED_IDENTITY_POLICY =
ConnectionStringBuilder.MANAGED_IDENTITY_AUTHENTICATION;
+ public static final AllowableValue AZURE_ENDPOINT = new
AllowableValue("servicebus.windows.net","Azure", "Servicebus endpoint for
general use");
+ public static final AllowableValue AZURE_CHINA_ENDPOINT = new
AllowableValue("servicebus.chinacloudapi.cn", "Azure China", "Servicebus
endpoint for China");
+ public static final AllowableValue AZURE_GERMANY_ENDPOINT = new
AllowableValue("servicebus.cloudapi.de", "Azure Germany", "Servicebus endpoint
for Germany");
+ public static final AllowableValue AZURE_US_GOV_ENDPOINT = new
AllowableValue("servicebus.usgovcloudapi.net", "Azure US Government",
"Servicebus endpoint for US Government");
Review comment:
Sorry I didn't notice this before. Since `GetAzureEventHub` already had
the `Service Bus Endpoint` property and used the leading `.`, switching to
values without the leading `.` will create an upgrade problem -- existing flows
will have an invalid configuration. So:
1. We could add the leading `.` back in and adjust whatever else needs to be
changed.
2. Remove the leading `.` as this currently does and note this in the
upgrade guidance.
Since it's mostly cosmetic and hidden by the description in the
`AllowableValues`, I prefer (1). What do you think?
```suggestion
public static final AllowableValue AZURE_ENDPOINT = new
AllowableValue(".servicebus.windows.net", "Azure", "Servicebus endpoint for
general use");
public static final AllowableValue AZURE_CHINA_ENDPOINT = new
AllowableValue(".servicebus.chinacloudapi.cn", "Azure China", "Servicebus
endpoint for China");
public static final AllowableValue AZURE_GERMANY_ENDPOINT = new
AllowableValue(".servicebus.cloudapi.de", "Azure Germany", "Servicebus endpoint
for Germany");
public static final AllowableValue AZURE_US_GOV_ENDPOINT = new
AllowableValue(".servicebus.usgovcloudapi.net", "Azure US Government",
"Servicebus endpoint for US Government");
```
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/eventhub/GetAzureEventHub.java
##########
@@ -81,6 +81,9 @@
@WritesAttribute(attribute = "eventhub.property.*", description = "The
application properties of this message. IE: 'application' would be
'eventhub.property.application'")
})
public class GetAzureEventHub extends AbstractProcessor {
+ private static final String TRANSIT_URI_FORMAT_STRING =
"amqps://%s%s/%s/ConsumerGroups/%s/Partitions/%s";
Review comment:
I think this is missing `.` with the current `AllowableValues`, but
depending on what we do with the other comment I made about the
`AllowableValues` the other transit uris will change and this one won't.
```suggestion
private static final String TRANSIT_URI_FORMAT_STRING =
"amqps://%s.%s/%s/ConsumerGroups/%s/Partitions/%s";
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
