You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/08/26 09:20:46 UTC
[GitHub] [iceberg] Fokko opened a new pull request #1385: Remove generated alerts
Fokko opened a new pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385
While working on upgrading the Avro dependency, I've noticed that there are a lot of warning, that can easily be resolved:
- Unused variables/methods
- Unnecessary parentheses
- Removed unchecked warning
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #1385: Remove generated alerts
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385#issuecomment-682256040
Looks like this is failing Java tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #1385: Remove generated alerts
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385#discussion_r479702145
##########
File path: spark3/src/main/java/org/apache/iceberg/spark/source/SparkBatchScan.java
##########
@@ -373,12 +372,5 @@ private Schema expectedSchema() {
}
return expectedSchema;
}
-
- private NameMapping nameMapping() {
- if (nameMapping == null) {
- this.nameMapping = NameMappingParser.fromJson(nameMappingString);
- }
- return nameMapping;
- }
Review 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
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a change in pull request #1385: Remove generated alerts
Posted by GitBox <gi...@apache.org>.
Fokko commented on a change in pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385#discussion_r479736574
##########
File path: spark3/src/main/java/org/apache/iceberg/spark/source/SparkBatchScan.java
##########
@@ -373,12 +372,5 @@ private Schema expectedSchema() {
}
return expectedSchema;
}
-
- private NameMapping nameMapping() {
- if (nameMapping == null) {
- this.nameMapping = NameMappingParser.fromJson(nameMappingString);
- }
- return nameMapping;
- }
Review comment:
I think we can remove this for now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko closed pull request #1385: Remove generated alerts
Posted by GitBox <gi...@apache.org>.
Fokko closed pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1385: Remove generated alerts
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385#discussion_r477339694
##########
File path: flink/src/main/java/org/apache/iceberg/flink/RowDataTaskWriterFactory.java
##########
@@ -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);
Review 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?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #1385: Remove generated alerts
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385#discussion_r479701913
##########
File path: flink/src/main/java/org/apache/iceberg/flink/RowDataTaskWriterFactory.java
##########
@@ -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);
Review 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).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a change in pull request #1385: Remove generated alerts
Posted by GitBox <gi...@apache.org>.
Fokko commented on a change in pull request #1385:
URL: https://github.com/apache/iceberg/pull/1385#discussion_r479736332
##########
File path: flink/src/main/java/org/apache/iceberg/flink/RowDataTaskWriterFactory.java
##########
@@ -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);
Review comment:
Good point, and I fully agree. I've created a Github issue: https://github.com/apache/iceberg/issues/1402
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org