[
https://issues.apache.org/jira/browse/HADOOP-19232?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17913556#comment-17913556
]
ASF GitHub Bot commented on HADOOP-19232:
-----------------------------------------
anujmodi2021 commented on code in PR #7272:
URL: https://github.com/apache/hadoop/pull/7272#discussion_r1917752527
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -234,6 +236,27 @@ public void initialize(URI uri, Configuration
configuration)
throw new
InvalidConfigurationValueException(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, ex);
}
+ /**
+ * Validates if the FixedSASTokenProvider is configured for non-HNS
accounts.
+ * If the namespace is not enabled and the FixedSASTokenProvider is not
configured,
+ * the filesystem is closed and an InvalidConfigurationValueException is
thrown.
+ *
+ * @throws InvalidConfigurationValueException if the namespace is not
enabled and the FixedSASTokenProvider is not configured.
+ */
+ try {
+ if (!getIsNamespaceEnabled(new TracingContext(initFSTracingContext))
+ && !abfsConfiguration.isFixedSASTokenProviderConfigured()) {
+ close();
Review Comment:
No need of close() here. FileSystem creation happening after all the checks
now.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -234,6 +236,27 @@ public void initialize(URI uri, Configuration
configuration)
throw new
InvalidConfigurationValueException(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, ex);
}
+ /**
Review Comment:
This should be a blocked comment instead of javadoc to avoid dangling
javadoc issues.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -234,6 +236,27 @@ public void initialize(URI uri, Configuration
configuration)
throw new
InvalidConfigurationValueException(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, ex);
}
+ /**
+ * Validates if the FixedSASTokenProvider is configured for non-HNS
accounts.
+ * If the namespace is not enabled and the FixedSASTokenProvider is not
configured,
+ * the filesystem is closed and an InvalidConfigurationValueException is
thrown.
+ *
+ * @throws InvalidConfigurationValueException if the namespace is not
enabled and the FixedSASTokenProvider is not configured.
+ */
+ try {
+ if (!getIsNamespaceEnabled(new TracingContext(initFSTracingContext))
Review Comment:
Better use `tryGetIsNamespaceEnabled()` here with proper exception handling.
Refer to:
https://github.com/apache/hadoop/blob/f38d7072566e88c77e47d1533e4be4c1bd98a06a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java#L242
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/exceptions/InvalidIngressServiceException.java:
##########
@@ -0,0 +1,51 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.contracts.exceptions;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+
+/**
+ * Exception thrown when an invalid ingress service is encountered.
+ *
+ * <p>This exception is used to indicate that the ingress service being used
+ * is not valid or supported. It extends the {@link AbfsRestOperationException}
+ * to provide additional context about the error.</p>
+ *
+ * @see AbfsRestOperationException
+ */
[email protected]
[email protected]
+public final class InvalidIngressServiceException extends
AbfsRestOperationException {
+
+ /**
+ * Constructs a new InvalidIngressServiceException with the specified
details.
+ *
+ * @param statusCode the HTTP status code
+ * @param errorCode the error code
+ * @param errorMessage the error message
+ * @param innerException the inner exception
+ */
+ public InvalidIngressServiceException(final int statusCode,
Review Comment:
May be I am missing something here, but I can see there are 2 constructors
for base exception and the one that excepts AbfsHttpOperation as a separate
parameter calls the `formatMessage()` which adds the req Id to exception
message.
Refering to:
https://github.com/apache/hadoop/blob/f38d7072566e88c77e47d1533e4be4c1bd98a06a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/exceptions/AbfsRestOperationException.java#L83
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -234,6 +236,27 @@ public void initialize(URI uri, Configuration
configuration)
throw new
InvalidConfigurationValueException(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, ex);
}
+ /**
+ * Validates if the FixedSASTokenProvider is configured for non-HNS
accounts.
Review Comment:
I feel like the comment here should be re-phrased. Its sounds like only
FixedSasTokenProvider can be used with FNS. It should clarify that if Auth Type
is SAS then user can only configure a fixed SAS Token and SASTokenProvider
should not be configured at all otherwise Fixed SAS Token won't be used.
Refer to
https://github.com/apache/hadoop/blob/f38d7072566e88c77e47d1533e4be4c1bd98a06a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java#L1225
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -234,6 +236,27 @@ public void initialize(URI uri, Configuration
configuration)
throw new
InvalidConfigurationValueException(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, ex);
}
+ /**
+ * Validates if the FixedSASTokenProvider is configured for non-HNS
accounts.
+ * If the namespace is not enabled and the FixedSASTokenProvider is not
configured,
+ * the filesystem is closed and an InvalidConfigurationValueException is
thrown.
+ *
+ * @throws InvalidConfigurationValueException if the namespace is not
enabled and the FixedSASTokenProvider is not configured.
+ */
+ try {
+ if (!getIsNamespaceEnabled(new TracingContext(initFSTracingContext))
+ && !abfsConfiguration.isFixedSASTokenProviderConfigured()) {
+ close();
+ throw new InvalidConfigurationValueException(
+ FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, UNAUTHORIZED_SAS);
+ }
+ } catch (AzureBlobFileSystemException ex) {
Review Comment:
We need better exception handling here.
We should first catch SASTokenProviderException and throw
InvalidConfigurationValueException() with original exception included as it
will tell why getSASTokenProvder() failed.
Then we should catch the AzureBlobFileSystemException that can be thrown
while determining account type.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java:
##########
@@ -279,6 +303,13 @@ public static ApiVersion getCurrentVersion() {
+ "non-hierarchical-namespace account:"
+ CPK_CONFIG_LIST;
+
+ /**
+ * Exception message on filesystem init if token-provider-auth-type configs
are provided
+ */
+ public static final String UNAUTHORIZED_SAS =
Review Comment:
Can be moved to AbfsErrors.java class
> ABFS: [FnsOverBlob] Implementing Ingress Support with various Fallback
> Handling
> -------------------------------------------------------------------------------
>
> Key: HADOOP-19232
> URL: https://issues.apache.org/jira/browse/HADOOP-19232
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Affects Versions: 3.4.0
> Reporter: Anuj Modi
> Assignee: Anmol Asrani
> Priority: Major
> Labels: pull-request-available
>
> Scope of this task is to refactor the AbfsOutputStream class to handle the
> ingress for DFS and Blob endpoint effectively.
> More details will be added soon.
> Perquisites for this Patch:
> 1. [HADOOP-19187] ABFS: [FnsOverBlob]Making AbfsClient Abstract for
> supporting both DFS and Blob Endpoint - ASF JIRA (apache.org)
> 2. [HADOOP-19226] ABFS: [FnsOverBlob]Implementing Azure Rest APIs on Blob
> Endpoint for AbfsBlobClient - ASF JIRA (apache.org)
> 3. [HADOOP-19207] ABFS: [FnsOverBlob]Response Handling of Blob Endpoint APIs
> and Metadata APIs - ASF JIRA (apache.org)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
