[
https://issues.apache.org/jira/browse/HADOOP-18675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17704324#comment-17704324
]
ASF GitHub Bot commented on HADOOP-18675:
-----------------------------------------
steveloughran commented on code in PR #5508:
URL: https://github.com/apache/hadoop/pull/5508#discussion_r1146691034
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/utils/TestCachedSASToken.java:
##########
@@ -78,6 +78,31 @@ public void testUpdateAndGet() throws IOException {
Assert.assertTrue(token3 == cachedToken);
}
+ @Test
+ public void testValidExpirationParsing() {
+ CachedSASToken cachedSasToken = new CachedSASToken();
+ String[] values = {
+ "2123-03-24T00:06:46Z", // sample timestamp from azure portal
+ "2124-03-30", // sample YYYY-MM-DD date format generated from az cli
+ "2125-03-30Z", // sample YYYY-MM-DD[offset] date format
+ };
+
+ for (String se : values) {
+ cachedSasToken.setForTesting(null, null);
+ String token = "se=" + se;
+
+ // set first time and ensure reference equality
+ cachedSasToken.update(token);
+ String cachedToken = cachedSasToken.get();
+ Assert.assertTrue(token == cachedToken);
Review Comment:
assertEquals(expected, actual) or AssertJ.asserThat(). thanks
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/CachedSASToken.java:
##########
@@ -114,8 +125,20 @@ private static OffsetDateTime getExpiry(String token) {
OffsetDateTime seDate = OffsetDateTime.MIN;
try {
seDate = OffsetDateTime.parse(seValue, DateTimeFormatter.ISO_DATE_TIME);
- } catch (DateTimeParseException ex) {
- LOG.error("Error parsing se query parameter ({}) from SAS.", seValue,
ex);
+ } catch (DateTimeParseException dateTimeException) {
+ try {
+ TemporalAccessor dt = isoDateMidnight.parseBest(seValue,
OffsetDateTime::from, LocalDateTime::from);
+ if (dt instanceof OffsetDateTime) {
+ seDate = (OffsetDateTime) dt;
+ } else if (dt instanceof LocalDateTime) {
+ seDate = ((LocalDateTime) dt).atOffset(ZoneOffset.UTC);
+ } else {
+ throw dateTimeException;
+ }
+ } catch (DateTimeParseException dateOnlyException) {
+ // log original exception
+ LOG.error("Error parsing se query parameter ({}) from SAS as
ISO_DATE_TIME or ISO_DATE.", seValue, dateTimeException);
Review Comment:
now, if there's a problem here it is still going to log a lot, isn't it?
might be best to use a LogExactlyOnce log here for the error line, and log
at debug the rest of the time
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/utils/TestCachedSASToken.java:
##########
@@ -78,6 +78,31 @@ public void testUpdateAndGet() throws IOException {
Assert.assertTrue(token3 == cachedToken);
}
+ @Test
+ public void testValidExpirationParsing() {
Review Comment:
is there a test for an invalid date time parsing now?
> CachedSASToken noisy log errors when SAS token has YYYY-MM-DD expiration
> ------------------------------------------------------------------------
>
> Key: HADOOP-18675
> URL: https://issues.apache.org/jira/browse/HADOOP-18675
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
> Reporter: Gil Cottle
> Priority: Major
> Labels: pull-request-available
>
> Error Description:
> When using SAS tokens with expiration dates in the format YYYY-MM-DD, a
> frequent error appears in the logs related to the date format. The error
> expects an ISO_DATE_TIME. See [existing
> implementation|https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/CachedSASToken.java#LL112-L119].
> The error is noisy in the logs, but does not cause issues.
> Example stacktrace:
> {code:java}
> 23/03/23 15:40:06 ERROR CachedSASToken: Error parsing se query parameter
> (2023-11-05) from SAS.
> java.time.format.DateTimeParseException: Text '2023-11-05' could not be
> parsed at index 10
> at
> java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:1949)
> at java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1851)
> at java.time.OffsetDateTime.parse(OffsetDateTime.java:402)
> at
> shaded.databricks.azurebfs.org.apache.hadoop.fs.azurebfs.utils.CachedSASToken.getExpiry(CachedSASToken.java:116)
> at
> shaded.databricks.azurebfs.org.apache.hadoop.fs.azurebfs.utils.CachedSASToken.update(CachedSASToken.java:168)
> at
> shaded.databricks.azurebfs.org.apache.hadoop.fs.azurebfs.services.AbfsInputStream.readRemote(AbfsInputStream.java:670)
> at
> shaded.databricks.azurebfs.org.apache.hadoop.fs.azurebfs.services.ReadBufferWorker.lambda$run$0(ReadBufferWorker.java:66)
> at
> com.databricks.common.SparkTaskIOMetrics.withTaskIOMetrics(SparkTaskIOMetrics.scala:43)
> at
> shaded.databricks.azurebfs.org.apache.hadoop.fs.azurebfs.services.ReadBufferWorker.run(ReadBufferWorker.java:65)
> at java.lang.Thread.run(Thread.java:750) {code}
> Desired Resolution:
> Expiration code can read YYYY-MM-DD format as well as existing format.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
