Detect Source Table Format#830
Conversation
sapienza88
changed the title
vinishjail97
left a comment
vinishjail97
left a comment
There was a problem hiding this comment.
For iceberg tables, the metadata can be in a different path and can't be detected by doing a prefix check? @sapienza88 Any idea where this change will be useful?
True, I fixed that. |
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.apache.commons</groupId> | ||
| <artifactId>commons-lang3</artifactId> |
There was a problem hiding this comment.
commons-lang3 added here but I don't see a usage in the diff — is it actually needed, or leftover?
| String[] args = new String[] {"--datasetConfig", configFile.getPath()}; | ||
| RunSync.main(args); | ||
| Path icebergMetadataPath = Paths.get(URI.create(table.getBasePath() + "/metadata")); | ||
| waitForNumIcebergCommits(icebergMetadataPath, 3); |
There was a problem hiding this comment.
Coverage gaps: only Hudi-source auto-detection is exercised. Please add detection tests for Delta and Iceberg sources, and for the "multiple formats detected" ambiguity branch (the case most likely to bite re-syncs). Also this asserts on commit count (waitForNumIcebergCommits(..., 3)) rather than data — a parameterized per-format test asserting the resolved format would be stronger.
| throw new UnsupportedOperationException("This class cannot be instantiated"); | ||
| } | ||
| // helper method to detect input format | ||
| public static String detectFormat(String pathStr, Configuration conf) throws IOException { |
There was a problem hiding this comment.
Public API method with no Javadoc (just an inline // helper comment) — please document the return value, the thrown exceptions, and the detection scope. Minor: the delta/hudi checks use the sanitized basePath, but HadoopTables.load(pathStr) uses the raw arg — use the sanitized path consistently.
Added more comments, the iceberg automatic detection using paths still has a problem. Can you take a look? |
2 participants
Important Read
What is the purpose of the pull request
(For example: This pull request implements the sync for delta format.)
Brief change log
(for example:)
Verify this pull request
(Please pick either of the following options)
This pull request is a trivial rework / code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)