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