Core: Support view metadata compression #8552
Merged
Conversation
This file contains 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
nastra
force-pushed
the
view-apis-with-compression
branch
from
311bb6b
to
8553226
Compare
rdblue
reviewed
Oct 2, 2023
core/src/test/java/org/apache/iceberg/view/TestViewMetadataParser.java
Outdated
Show resolved
Hide resolved
rdblue
reviewed
Oct 2, 2023
| @ValueSource(strings = {"none", "gzip"}) | ||
| public void metadataCompression(String codecName) throws IOException { | ||
| Codec codec = Codec.fromName(codecName); | ||
| String location = Paths.get(tmp.toString(), "v1" + getFileExtension(codec)).toString(); |
There was a problem hiding this comment.
I don't think that this should rely on getFileExtension. It's fine that it is used by BaseViewOperations to name the file, but there's no need to do that here. Instead, I think this should be parameterized by file name (v1.metadata.json and v1.gz.metadata.json) so that it is obvious what happens based on the file name in the test. Otherwise this is testing that the extension produced by getFileExtension triggers some behavior in the parser, rather than a specific extension triggers behavior in the parser.
There was a problem hiding this comment.
I was aligning this test with how compression was tested for table metadata in
but I've changed it to have explicit file names
nastra
force-pushed
the
view-apis-with-compression
branch
from
8553226
to
2eb49de
Compare
nastra
force-pushed
the
view-apis-with-compression
branch
from
2eb49de
to
243a4b9
Compare
rdblue
approved these changes
Oct 11, 2023
|
Thanks, @nastra! |
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.
No description provided.