Add expiration to stateful test #868
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcherian
force-pushed
the
stateful-expiry
branch
2 times, most recently
from
f4df554 to
c155d59
Compare
dcherian
commented
Mar 27, 2025
dcherian
force-pushed
the
stateful-expiry
branch
from
8022400 to
98dfdf7
Compare
dcherian
changed the title
expire_snapshots to stateful test
dcherian
force-pushed
the
stateful-expiry
branch
from
85fc37b to
876229f
Compare
dcherian
commented
Mar 28, 2025
dcherian
commented
Mar 28, 2025
| repo.lookup_snapshot(snap).written_at + timedelta(microseconds=1) | ||
| ) | ||
| with pytest.raises(ValueError): | ||
| repo.lookup_snapshot(snap) |
dcherian
force-pushed
the
stateful-expiry
branch
from
c347a32 to
1e309bd
Compare
dcherian
commented
Mar 28, 2025
| delete_expired_tags=True, | ||
| ) | ||
| assert expired == {a, b} | ||
| assert repo.list_tags() == {} |
dcherian
force-pushed
the
stateful-expiry
branch
from
1e309bd to
5da493d
Compare
dcherian
force-pushed
the
stateful-expiry
branch
3 times, most recently
from
052e2c6 to
823a46a
Compare
dcherian
force-pushed
the
stateful-expiry
branch
from
823a46a to
63663b1
Compare
Contributor
Author
|
OK I think this is finally done 🥳 . |
paraseba
approved these changes
Mar 31, 2025
| class CommitModel: | ||
| id: str | ||
| written_at: datetime.datetime | ||
| state: dict[str, Any] |
Collaborator
There was a problem hiding this comment.
I was curious about what the state was gonna be. Should we call this store?
| tag_iter = map(operator.attrgetter("commit_id"), self.tags.values()) | ||
| return itertools.chain(self.branches.values(), tag_iter) | ||
|
|
||
| def expire_snapshots( |
Collaborator
There was a problem hiding this comment.
I think all of this is great, but, we are re-implementing the algorithms in the tests, potentially with the same bugs we are blind to. I wonder if in the future we can define properties of the GCed or Expired repo, and try to track less in the model.
Contributor
Author
There was a problem hiding this comment.
I landed up here because I'm doing an exact equality check for snapshot ids that were expired because only expires snaphsots that are reachable.
I guess I can search the "forest" and assert that set(IC expired IDs) < set(all possible expirable IDs)
dcherian
added a commit
that referenced
this pull request
Apr 3, 2025
* main: Fix `Diff` python typing (#890) Fail when creating Storage for Tigris using s3_compatible (#889) Disallow tag/branch creation with non-existing snapshot (#888) Log errors during listing and deleting of objects (#886) Rust integration tests can run in more object stores. (#884) Update pyo3. (#885) Add expiration to stateful test (#868)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #867
Adds both
expire_snapshotsandgarbage_collect.expire_snapshotsis basically an independent reimplementation of the rust logic.garbage_collectis very simple, and only deletes expires snapshots.