Remove generated alerts #1385
Remove generated alerts #1385
Conversation
Fokko
force-pushed
the
fd-remove-alerts
branch
from
1eea84f to
282a3fb
Compare
Fokko
force-pushed
the
fd-remove-alerts
branch
from
282a3fb to
d94727e
Compare
| @@ -127,7 +126,7 @@ private FlinkFileAppenderFactory(Schema schema, RowType flinkSchema, Map<String, | |||
| @Override | |||
| public FileAppender<RowData> newAppender(OutputFile outputFile, FileFormat format) { | |||
| // TODO MetricsConfig will be used for building parquet RowData writer. | |||
| MetricsConfig metricsConfig = MetricsConfig.fromProperties(props); | |||
| // MetricsConfig metricsConfig = MetricsConfig.fromProperties(props); | |||
There was a problem hiding this comment.
My only question here is why not just delete the line and todo if we aren't using them and just have a github issue instead?
There was a problem hiding this comment.
I agree here. A github issue is more likely to be picked up and worked on over a TODO in the codebase.... though I'm not opposed to keeping the TODO so people don't accidentally overly refactor away from the possibility (if that's a potential concern).
There was a problem hiding this comment.
Good point, and I fully agree. I've created a Github issue: #1402
|
Looks like this is failing Java tests. |
| this.nameMapping = NameMappingParser.fromJson(nameMappingString); | ||
| } | ||
| return nameMapping; | ||
| } |
There was a problem hiding this comment.
I think there might be a PR open related to this. However, I don't see this particular function used: https://github.com/apache/iceberg/pull/1399/files
There was a problem hiding this comment.
I think we can remove this for now.
…into fd-remove-alerts
While working on upgrading the Avro dependency, I've noticed that there are a lot of warning, that can easily be resolved:
Makes the output a bit less verbose :)