wypoon commented on code in PR #6799:
URL: https://github.com/apache/iceberg/pull/6799#discussion_r1116245264
##########
core/src/main/java/org/apache/iceberg/avro/Avro.java:
##########
@@ -69,7 +69,7 @@
public class Avro {
private Avro() {}
- private enum Codec {
+ public enum Codec {
Review Comment:
I'm afraid I don't understand what you're suggesting.
The Map in `TestTableBase` that is now named `AVRO_CODEC_NAME_MAPPING`
originally used plain Strings, and @nastra advocated using `Avro.Codec` values,
but in order to do that, I had to make the enum public. To be clear,
`AVRO_CODEC_NAME_MAPPING` is still a `Map<String, String>`:
```
static final Map<String, String> AVRO_CODEC_NAME_MAPPING =
ImmutableMap.<String, String>builder()
.put(Avro.Codec.UNCOMPRESSED.name(), DataFileConstants.NULL_CODEC)
.put(Avro.Codec.ZSTD.name(), DataFileConstants.ZSTANDARD_CODEC)
.put(Avro.Codec.GZIP.name(), DataFileConstants.DEFLATE_CODEC)
.build();
```
Referencing the `Avro.Codec` values is simply to make clear that those
Strings are codec names.
I'm fine with using plain Strings instead. Is that what you're suggesting?
I don't understand your comment about adding a `fromName` method. Do you
mean to add a (static) method to `Avro.Codec` that takes a String and returns
the enum? How would I be able to use it, if `Avro.Codec` remains private? I
can't call it from outside `Avro`, because the enum is private, and I can't use
the return value, for the same reason.
I do agree that `Avro.Codec` should be private, because the only use of it
is for switching on the values within a private static method in `Avro`, to
return the appropriate `org.apache.avro.file.CodecFactory`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
