saxenapranav commented on code in PR #6847: URL: https://github.com/apache/hadoop/pull/6847#discussion_r1630837840
########## hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java: ########## @@ -202,21 +202,25 @@ private AbfsClient(final URL baseUrl, this.isMetricCollectionStopped = new AtomicBoolean(false); this.metricAnalysisPeriod = abfsConfiguration.getMetricAnalysisTimeout(); this.metricIdlePeriod = abfsConfiguration.getMetricIdleTimeout(); - if (!metricFormat.toString().equals("")) { - isMetricCollectionEnabled = true; - abfsCounters.initializeMetrics(metricFormat); + if (!metricFormat.toString().equals(EMPTY_STRING)) { String metricAccountName = abfsConfiguration.getMetricAccount(); - int dotIndex = metricAccountName.indexOf(AbfsHttpConstants.DOT); - if (dotIndex <= 0) { - throw new InvalidUriException( - metricAccountName + " - account name is not fully qualified."); - } String metricAccountKey = abfsConfiguration.getMetricAccountKey(); - try { - metricSharedkeyCredentials = new SharedKeyCredentials(metricAccountName.substring(0, dotIndex), - metricAccountKey); - } catch (IllegalArgumentException e) { - throw new IOException("Exception while initializing metric credentials " + e); + if (!metricAccountName.equals(EMPTY_STRING) && !metricAccountKey.equals(EMPTY_STRING)) { Review Comment: would be better if we can use StringUtils.isNotEmpty(), as it would null and empty checks. ########## hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java: ########## @@ -202,21 +202,25 @@ private AbfsClient(final URL baseUrl, this.isMetricCollectionStopped = new AtomicBoolean(false); this.metricAnalysisPeriod = abfsConfiguration.getMetricAnalysisTimeout(); this.metricIdlePeriod = abfsConfiguration.getMetricIdleTimeout(); - if (!metricFormat.toString().equals("")) { - isMetricCollectionEnabled = true; - abfsCounters.initializeMetrics(metricFormat); + if (!metricFormat.toString().equals(EMPTY_STRING)) { String metricAccountName = abfsConfiguration.getMetricAccount(); - int dotIndex = metricAccountName.indexOf(AbfsHttpConstants.DOT); - if (dotIndex <= 0) { - throw new InvalidUriException( - metricAccountName + " - account name is not fully qualified."); - } String metricAccountKey = abfsConfiguration.getMetricAccountKey(); - try { - metricSharedkeyCredentials = new SharedKeyCredentials(metricAccountName.substring(0, dotIndex), - metricAccountKey); - } catch (IllegalArgumentException e) { - throw new IOException("Exception while initializing metric credentials " + e); + if (!metricAccountName.equals(EMPTY_STRING) && !metricAccountKey.equals(EMPTY_STRING)) { + isMetricCollectionEnabled = true; + abfsCounters.initializeMetrics(metricFormat); + int dotIndex = metricAccountName.indexOf(AbfsHttpConstants.DOT); + if (dotIndex <= 0) { + throw new InvalidUriException( + metricAccountName + " - account name is not fully qualified."); + } + try { + metricSharedkeyCredentials = new SharedKeyCredentials( + metricAccountName.substring(0, dotIndex), + metricAccountKey); + } catch (IllegalArgumentException e) { + throw new IOException( + "Exception while initializing metric credentials " + e); Review Comment: throw new IOException(message, e) would be better as the consumer of this exception can get the inner-exception. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org