[
https://issues.apache.org/jira/browse/HADOOP-17725?focusedWorklogId=600434&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-600434
]
ASF GitHub Bot logged work on HADOOP-17725:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 21/May/21 15:26
Start Date: 21/May/21 15:26
Worklog Time Spent: 10m
Work Description: sadikovi commented on a change in pull request #3041:
URL: https://github.com/apache/hadoop/pull/3041#discussion_r637006602
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java
##########
@@ -361,6 +366,56 @@ public void testAccessTokenProviderPrecedence()
testGlobalAndAccountOAuthPrecedence(abfsConf, null, AuthType.OAuth);
}
+ @Test
+ public void testConfigPropNotFound() throws Exception {
+ final String accountName = "account";
+
+ final Configuration conf = new Configuration();
+ final AbfsConfiguration abfsConf = new AbfsConfiguration(conf,
accountName);
+
+ setAuthConfig(abfsConf, true, AuthType.OAuth);
+ abfsConf.unset(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT + "." + accountName);
Review comment:
Hmm... Can you create some kind of for loop to iterate over the keys,
otherwise, the code is duplicated quite a bit. I would also suggest creating
separate tests for each.
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/ClientCredsTokenProvider.java
##########
@@ -38,19 +42,27 @@
private static final Logger LOG =
LoggerFactory.getLogger(AccessTokenProvider.class);
-
public ClientCredsTokenProvider(final String authEndpoint,
- final String clientId, final String
clientSecret) {
-
- Preconditions.checkNotNull(authEndpoint, "authEndpoint");
- Preconditions.checkNotNull(clientId, "clientId");
- Preconditions.checkNotNull(clientSecret, "clientSecret");
+ final String clientId, final String clientSecret)
+ throws AzureBlobFileSystemException {
+ validateClientCredsTokenProvider(
Review comment:
That is not going to work. You probably want to keep non-null check
here, since this class does not depend on Hadoop configuration.
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/ClientCredsTokenProvider.java
##########
@@ -38,19 +42,27 @@
private static final Logger LOG =
LoggerFactory.getLogger(AccessTokenProvider.class);
-
public ClientCredsTokenProvider(final String authEndpoint,
- final String clientId, final String
clientSecret) {
-
- Preconditions.checkNotNull(authEndpoint, "authEndpoint");
- Preconditions.checkNotNull(clientId, "clientId");
- Preconditions.checkNotNull(clientSecret, "clientSecret");
+ final String clientId, final String clientSecret)
+ throws AzureBlobFileSystemException {
+ validateClientCredsTokenProvider(
+ ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT,
authEndpoint);
+ validateClientCredsTokenProvider(
+ ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID, clientId);
+ validateClientCredsTokenProvider(
+ ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET, clientSecret);
this.authEndpoint = authEndpoint;
this.clientId = clientId;
this.clientSecret = clientSecret;
}
+ private void validateClientCredsTokenProvider(final String configName,
Review comment:
IMHO, it is better to move it to AbfsConfiguration.java. It will allow
you to cover other token providers there as well.
--
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 600434)
Time Spent: 20m (was: 10m)
> Improve error message for token providers in ABFS
> -------------------------------------------------
>
> Key: HADOOP-17725
> URL: https://issues.apache.org/jira/browse/HADOOP-17725
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs/azure, hadoop-thirdparty
> Affects Versions: 3.3.0
> Reporter: Ivan
> Assignee: Viraj Jasani
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> It would be good to improve error messages for token providers in ABFS.
> Currently, when a configuration key is not found or mistyped, the error is
> not very clear on what went wrong. It would be good to indicate that the key
> was required but not found in Hadoop configuration when creating a token
> provider.
> For example, when running the following code:
> {code:java}
> import org.apache.hadoop.conf._
> import org.apache.hadoop.fs._
> val conf = new Configuration()
> conf.set("fs.azure.account.auth.type", "OAuth")
> conf.set("fs.azure.account.oauth.provider.type",
> "org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider")
> conf.set("fs.azure.account.oauth2.client.id", "my-client-id")
> //
> conf.set("fs.azure.account.oauth2.client.secret.my-account.dfs.core.windows.net",
> "my-secret")
> conf.set("fs.azure.account.oauth2.client.endpoint", "my-endpoint")
> val path = new Path("abfss://[email protected]/")
> val fs = path.getFileSystem(conf)
> fs.getFileStatus(path){code}
> The following exception is thrown:
> {code:java}
> TokenAccessProviderException: Unable to load OAuth token provider class.
> ...
> Caused by: UncheckedExecutionException: java.lang.NullPointerException:
> clientSecret
> ...
> Caused by: NullPointerException: clientSecret {code}
> which does not tell what configuration key was not loaded.
>
> IMHO, it would be good if the exception was something like this:
> {code:java}
> TokenAccessProviderException: Unable to load OAuth token provider class.
> ...
> Caused by: ConfigurationPropertyNotFoundException: Configuration property
> fs.azure.account.oauth2.client.secret not found. {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
