Skip to content

Conversation

@TomNicholas
Copy link
Member

@TomNicholas TomNicholas commented Mar 7, 2025

@TomNicholas TomNicholas added the Icechunk 🧊 Relates to Icechunk library / spec label Mar 7, 2025
Comment on lines 332 to 335
# Icechunk checksums currently store with second precision, so we need to make sure
# the checksum_date is at least one second in the future
timestamp_checksum = datetime.now(timezone.utc) + timedelta(seconds=1)

Copy link
Member Author

Choose a reason for hiding this comment

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

@mpiannucci I feel like this +1 second business should be in the test, rather than in the business logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I agree

Copy link
Member Author

Choose a reason for hiding this comment

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

We actually came to the opposite conclusion on a call just now. The long comments in the code hopefully explain our reasoning.

@TomNicholas
Copy link
Member Author

The NotImplementedError is #596, but the other failure is real, either a problem with the business logic or tests in this PR

@TomNicholas TomNicholas changed the base branch from main to develop June 26, 2025 18:53
@TomNicholas TomNicholas merged commit afb1336 into zarr-developers:develop Jun 26, 2025
11 checks passed
@TomNicholas TomNicholas deleted the staleness-check-opt-out branch June 26, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Icechunk 🧊 Relates to Icechunk library / spec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Virtual chunk staleness check should be opt-out, not opt-in

2 participants