[Storage] Refactor on getCommits in UC Delta token-based client#6899
[Storage] Refactor on getCommits in UC Delta token-based client#6899ChengJiX wants to merge 1 commit into
Conversation
| return new GetCommitsResponse(commits, latestTableVersion); | ||
| Optional<io.delta.storage.commit.uniform.UniformMetadata> storageUniform = | ||
| toStorageUniformMetadata(response.getUniform()); | ||
| return new GetCommitsFromLoadTableResponse(commits, latestTableVersion, storageUniform); |
There was a problem hiding this comment.
Hi @ChengJiX , My question is, as we will always return the general GetCommitsResponse in the UCClient API, then how could we use the Optional<UniformMetadata> uniformMetadata that defined in the concrete GetCommitsFromLoadTableResponse implementation ?
do we need to cast the GetCommitsResponse to GetCommitsFromLoadTableResponse so that we can access the uniformMetadata ?
| @@ -0,0 +1,54 @@ | |||
| /* | |||
| * Copyright (2021) The Delta Lake Project Authors. | |||
There was a problem hiding this comment.
nit: The new class will usually use the 2026 copyright.
| .stream() | ||
| .sorted(Comparator.comparingLong(Commit::getVersion)) | ||
| .collect(Collectors.toList()); | ||
| return new GetCommitsResponse(sortedCommits, resp.getLatestTableVersion()); |
There was a problem hiding this comment.
My question is: when will we use the GetCommitsFromLoadTableResponse in the code path ? I still did not see the usage that the place to use the GetCommitsFromLoadTableResponse in this PR, seems like this PR is extending the abstraction.
There was a problem hiding this comment.
Hi @openinx, thanks for review! It would be in next PR. It would be used by downstream consumer like this:
val uniformMetadataOpt = unbackfilledCommitsResponse match {
case r: GetCommitsFromLoadTableResponse => Option(r.getUniformMetadata).flatMap(_.toScala)
case _ => None
}
More specifically, for UniForm conflict resolution, it would be used in listDeltaCompactedDeltaCheckpointFilesAndLatestChecksumFile
2 participants
Description
Refactor on
getCommitsin UC Delta token-based client such that it returns a more generalGetCommitsFromLoadTableResponsethat can be extended in future to carry more metadata. The currentGetCommitsResponseis a narrow interface that would drop additional metadata fromloadTableHow was this patch tested?
Existing tests