rdblue commented on code in PR #12298:
URL: https://github.com/apache/iceberg/pull/12298#discussion_r2373029124


##########
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:
   I don't think that this should not rename methods like `setAll`. I don't see 
the purpose of changing the name and it will require unnecessary changes, right?



-- 
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]

Reply via email to