abhishekagarwal87 commented on code in PR #15523:
URL: https://github.com/apache/druid/pull/15523#discussion_r1440138455
##########
extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureUtils.java:
##########
@@ -40,11 +40,26 @@
public class AzureUtils
{
+ public static final String DEFAULT_AZURE_ENDPOINT_SUFFIX =
"core.windows.net";
@VisibleForTesting
static final String AZURE_STORAGE_HOST_ADDRESS = "blob.core.windows.net";
+ public static final String DEFAULT_AZURE_BLOB_STORAGE_ENDPOINT_SUFFIX =
"blob." + DEFAULT_AZURE_ENDPOINT_SUFFIX;
Review Comment:
how is this different from `AZURE_STORAGE_HOST_ADDRESS`? We can use that
name itself instead of adding this one.
##########
extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureUtils.java:
##########
@@ -40,11 +40,26 @@
public class AzureUtils
{
+ public static final String DEFAULT_AZURE_ENDPOINT_SUFFIX =
"core.windows.net";
@VisibleForTesting
static final String AZURE_STORAGE_HOST_ADDRESS = "blob.core.windows.net";
+ public static final String DEFAULT_AZURE_BLOB_STORAGE_ENDPOINT_SUFFIX =
"blob." + DEFAULT_AZURE_ENDPOINT_SUFFIX;
+ private final String blobStorageEndpointSuffix;
+
+ /**
+ * Creates an AzureUtils object with the blob storage endpoint suffix.
+ *
+ * @param blobStorageEndpointSuffix the blob storage endpoint, like
<code>"blob.core.windows.net"</code>,
+ *
<code>"blob.core.chinacloudapi.cn"</code> or
+ *
<code>"blob.core.usgovcloudapi.net</code>"
+ */
+ public AzureUtils(String blobStorageEndpointSuffix)
+ {
+ this.blobStorageEndpointSuffix = blobStorageEndpointSuffix;
+ }
// The azure storage hadoop access pattern is:
- // wasb[s]://<containername>@<accountname>.blob.core.windows.net/<path>
+ // wasb[s]://<containername>@<accountname>.<blobStorageEndpointSuffix>/<path>
Review Comment:
```suggestion
// wasb[s]://<containername>@<accountname>.blob.<endpointSuffix>/<path>
```
##########
extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureAccountConfig.java:
##########
@@ -94,9 +108,13 @@ public String getSharedAccessStorageToken()
return sharedAccessStorageToken;
}
- @SuppressWarnings("unused") // Used by Jackson deserialization?
- public void setSharedAccessStorageToken(String sharedAccessStorageToken)
+ public String getEndpointSuffix()
{
- this.sharedAccessStorageToken = sharedAccessStorageToken;
+ return endpointSuffix;
+ }
+
+ public String getBlobStorageEndpointSuffix()
Review Comment:
isn't it the whole endpoint itself? why are we calling this method
`getBlobStorageEndpointSuffix` instead of `getBlobStorageEndpoint`
##########
extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureUtils.java:
##########
@@ -40,11 +40,26 @@
public class AzureUtils
{
+ public static final String DEFAULT_AZURE_ENDPOINT_SUFFIX =
"core.windows.net";
@VisibleForTesting
static final String AZURE_STORAGE_HOST_ADDRESS = "blob.core.windows.net";
+ public static final String DEFAULT_AZURE_BLOB_STORAGE_ENDPOINT_SUFFIX =
"blob." + DEFAULT_AZURE_ENDPOINT_SUFFIX;
+ private final String blobStorageEndpointSuffix;
+
+ /**
+ * Creates an AzureUtils object with the blob storage endpoint suffix.
+ *
+ * @param blobStorageEndpointSuffix the blob storage endpoint, like
<code>"blob.core.windows.net"</code>,
+ *
<code>"blob.core.chinacloudapi.cn"</code> or
+ *
<code>"blob.core.usgovcloudapi.net</code>"
+ */
+ public AzureUtils(String blobStorageEndpointSuffix)
Review Comment:
since its a utils class, lets not add a constructor here.
##########
extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentPuller.java:
##########
@@ -59,7 +64,7 @@ FileUtils.FileCopyResult getSegmentFiles(
"Loading container: [%s], with blobPath: [%s] and outDir: [%s]",
containerName, blobPath, outDir
);
- final String actualBlobPath =
AzureUtils.maybeRemoveAzurePathPrefix(blobPath);
+ final String actualBlobPath =
azureUtils.maybeRemoveAzurePathPrefix(blobPath);
Review Comment:
you could pass the endpoint suffix as an argument than passing it in the
AzureUtils constructor
##########
docs/development/extensions-core/azure.md:
##########
@@ -40,5 +40,6 @@ To use this Apache Druid extension,
[include](../../configuration/extensions.md#
|`druid.azure.protocol`|the protocol to use|http or https|https|
|`druid.azure.maxTries`|Number of tries before canceling an Azure operation.|
|3|
|`druid.azure.maxListingLength`|maximum number of input files matching a given
prefix to retrieve at a time| |1024|
+|`druid.azure.endpointSuffix`|the endpoint suffix to use.|Examples:
`core.windows.net`, `core.usgovcloudapi.net`|`core.windows.net`|
Review Comment:
can you add some more colour here? when would one want to override this?
Pointing to external documentation is fine too.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
