pvary commented on code in PR #12298:
URL: https://github.com/apache/iceberg/pull/12298#discussion_r2375680325
##########
core/src/main/java/org/apache/iceberg/avro/Avro.java:
##########
@@ -100,74 +110,82 @@ public static WriteBuilder write(OutputFile file) {
return new WriteBuilder(file);
}
+ /**
+ * @deprecated Since 1.10.0, will be removed in 1.11.0. Use {@link
+ * FormatModelRegistry#writeBuilder(FileFormat, Class,
EncryptedOutputFile)} instead.
+ */
+ @Deprecated
public static WriteBuilder write(EncryptedOutputFile file) {
return new WriteBuilder(file.encryptingOutputFile());
}
+ /**
+ * @deprecated Since 1.10.0, will be removed in 1.11.0. Use {@link
+ * FormatModelRegistry#writeBuilder(FileFormat, Class,
EncryptedOutputFile)} instead.
+ */
+ @Deprecated
public static class WriteBuilder implements InternalData.WriteBuilder {
- private final OutputFile file;
- private final Map<String, String> config = Maps.newHashMap();
- private final Map<String, String> metadata = Maps.newLinkedHashMap();
- private org.apache.iceberg.Schema schema = null;
- private String name = "table";
- private Function<Schema, DatumWriter<?>> createWriterFunc = null;
- private boolean overwrite;
- private MetricsConfig metricsConfig;
- private Function<Map<String, String>, Context> createContextFunc =
Context::dataContext;
+ private final WriteBuilderImpl<?, ?> impl;
private WriteBuilder(OutputFile file) {
- this.file = file;
+ this.impl = new WriteBuilderImpl<>(file);
}
public WriteBuilder forTable(Table table) {
- schema(table.schema());
- setAll(table.properties());
- metricsConfig(MetricsConfig.forTable(table));
+ impl.schema(table.schema())
+ .set(table.properties())
+ .metricsConfig(MetricsConfig.forTable(table));
return this;
}
@Override
public WriteBuilder schema(org.apache.iceberg.Schema newSchema) {
- this.schema = newSchema;
+ impl.schema(newSchema);
return this;
}
@Override
public WriteBuilder named(String newName) {
- this.name = newName;
+ impl.name = newName;
return this;
}
- public WriteBuilder createWriterFunc(Function<Schema, DatumWriter<?>>
writerFunction) {
- this.createWriterFunc = writerFunction;
+ public WriteBuilder createWriterFunc(Function<Schema, DatumWriter<?>>
newWriterFunction) {
+ Preconditions.checkState(
+ impl.writerFunction == null, "Cannot set multiple writer builder
functions");
+ if (newWriterFunction != null) {
+ impl.createWriterFunc = s -> (DatumWriter<Object>)
newWriterFunction.apply(s);
+ } else {
+ impl.createWriterFunc = null;
+ }
return this;
}
@Override
public WriteBuilder set(String property, String value) {
- config.put(property, value);
+ impl.set(property, value);
return this;
}
public WriteBuilder setAll(Map<String, String> properties) {
- config.putAll(properties);
+ impl.set(properties);
Review Comment:
Collected the mapping between the old builder methods and the new builder
methods in a doc:
https://docs.google.com/spreadsheets/d/1cBwyrO9-0x-OgquNhfBpkjfF__VRZgKBqujCnlAkXeY/edit?gid=0#gid=0
We can revisit all of them, and decide what to do.
As I have answered on another comment, we have changed it for consistency's
shake.
Currently we have:
- set(String key, String value)
- config(String property, String value)
- setAll(Map<String, String> properties)
- meta(String property, String value)
- metadata(String property, String value)
We have consolidated it to:
- set(String key, String value)
- set(Map<String, String> properties)
- meta(String property, String value)
- meta(Map<String, String> properties)
I think this is way cleaner than the original version.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
