-
Notifications
You must be signed in to change notification settings - Fork 2k
[Kernel-Spark][DSv2] Implement UCManagedSnapshotManager operations using UCCatalogManagedClient #5677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[Kernel-Spark][DSv2] Implement UCManagedSnapshotManager operations using UCCatalogManagedClient #5677
Conversation
Signed-off-by: Timothy Wang <[email protected]>
Signed-off-by: Timothy Wang <[email protected]>
Signed-off-by: Timothy Wang <[email protected]>
Signed-off-by: TimothyW553 <[email protected]>
Signed-off-by: Timothy Wang <[email protected]>
Signed-off-by: Timothy Wang <[email protected]>
Signed-off-by: Timothy Wang <[email protected]>
Signed-off-by: TimothyW553 <[email protected]>
Signed-off-by: TimothyW553 <[email protected]>
...src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UCManagedTableSnapshotManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why two tests files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Java one is for constructor validation. The other uses Scala because there are a number of scala utils that help with testing such as UCCatalogManagedTestUtils and InMemoryUCClient (written in Scala) so it is easier to write the bulk of the tests in Scala.
This is the reason, but we could
- Get rid of the Java tests (as they are just simple constructor tests)
- Test the Java code and consolidate them in the Scala test suite.
🥞 Stacked PR
Use this link to review incremental changes.
Which Delta project/connector is this regarding?
Description
This PR implements the wireframe introduced in the previous PR of the stack. For this specific
UCManagedTableSnapshotManager, it delegates snapshot loading operations to theUCCatalogCommitClient.getActiveCommitAtTime(t)loads the latest snapshot, gets its commits, and uses those along withDeltaHistoryManager.getActiveCommitAtTimestamp(t, catalogCommits, ...)to get the appropriate commit at timet.How was this patch tested?
Locally and CI.
Does this PR introduce any user-facing changes?
No.