pvary commented on code in PR #12366: URL: https://github.com/apache/iceberg/pull/12366#discussion_r1965827489
########## data/src/main/java/org/apache/iceberg/data/GenericAppenderFactory.java: ########## @@ -91,7 +118,13 @@ public FileAppender<Record> newAppender(OutputFile outputFile, FileFormat fileFo @Override public FileAppender<Record> newAppender( EncryptedOutputFile encryptedOutputFile, FileFormat fileFormat) { - MetricsConfig metricsConfig = MetricsConfig.fromProperties(config); + MetricsConfig metricsConfig; + if (table == null) { + metricsConfig = MetricsConfig.fromProperties(config); + } else { + metricsConfig = MetricsConfig.forTable(table); + } + Review Comment: This is a bit questionable for me. Do we really want to disregard the config properties specifically set by the `set`/`setAll` method if the table has provided? We should decide on the expected behavior and make sure that we don't get wrong configuration. Maybe by throwing an exception in the set methods when the table is set? -- 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