Skip to content

Change last update timestamp granularity of GCS objects from seconds to milliseconds#16083

Merged
kfaraz merged 5 commits intoapache:masterfrom
gargvishesh:42838-fix-recent-task-logs-deleted
Mar 9, 2024
Merged

Change last update timestamp granularity of GCS objects from seconds to milliseconds#16083
kfaraz merged 5 commits intoapache:masterfrom
gargvishesh:42838-fix-recent-task-logs-deleted

Conversation

@gargvishesh
Copy link
Copy Markdown
Contributor

The previously used GCS API client library returned last update time for objects directly in milliseconds. The new library returns it in OffsetDateTime format which was being converted to seconds and stored against the object. This fix converts the time back to ms before storing it.

@kfaraz
Copy link
Copy Markdown
Contributor

kfaraz commented Mar 8, 2024

@gargvishesh , thanks for the fix. GoogleStorageTest might need to be updated too.
Can we also add a test that ensures that the correct millisecond value is being used now instead of second?


}
@Test
public void testGetMetadataMismatch() throws IOException
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this test for?

Copy link
Copy Markdown
Contributor

@kfaraz kfaraz Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think it is the same as the above test testGetMetadataMismatch() except that it verifies that the timestamp is not in seconds .

@gargvishesh , I think the previous test testGetMetadata() is enough.

@cryptoe cryptoe added this to the 29.0.1 milestone Mar 8, 2024

}
@Test
public void testGetMetadataMismatch() throws IOException
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think it is the same as the above test testGetMetadataMismatch() except that it verifies that the timestamp is not in seconds .

@gargvishesh , I think the previous test testGetMetadata() is enough.

@cryptoe
Copy link
Copy Markdown
Contributor

cryptoe commented Mar 12, 2024

Caused because of : #15398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants