You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ajantha-bhat (via GitHub)" <gi...@apache.org> on 2023/04/11 16:40:38 UTC

[GitHub] [iceberg] ajantha-bhat opened a new pull request, #7321: Build: Fix flaky checkstyle failure

ajantha-bhat opened a new pull request, #7321:
URL: https://github.com/apache/iceberg/pull/7321

   https://github.com/apache/iceberg/pull/7024 didn't really fix the issue. 
   
   After analyzing the dependency tree found that `com.palantir.baseline-checkstyle` plugin will always use the checkstyle version from `com.palantir.baseline:gradle-baseline-java:4.42.0` (which is 9.1) even though we override it with some other version (which is 9.3).
   
   So, the solution is to use some other check style plugin with the required checkstyle version 9.3.
   Using the gradle checkstyle plugin now as  `com.palantir.baseline-checkstyle` was also a simple wrapper around that.
   
   - Executed the dependency tree `gradle iceberg-api:dependencies` and confirmed it is using checkstyle 9.3 which has the fix of OOM issue which is causing build flakiness.
   - Fixed the check style errors reported from 9.3 version (which was not present in 9.1 version)
    
   


-- 
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-unsubscribe@iceberg.apache.org

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] ajantha-bhat commented on pull request #7321: Build: Fix flaky checkstyle failure

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #7321:
URL: https://github.com/apache/iceberg/pull/7321#issuecomment-1503754871

   cc: @nastra, @Fokko, @stevenzwu, @jackye1995 , @XN137    


-- 
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-unsubscribe@iceberg.apache.org

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] nastra commented on a diff in pull request #7321: Build: Fix flaky checkstyle issue

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7321:
URL: https://github.com/apache/iceberg/pull/7321#discussion_r1163102594


##########
baseline.gradle:
##########
@@ -33,7 +33,11 @@ subprojects {
   // ready to enforce linting on.
   apply plugin: 'org.inferred.processors'
   if (!project.hasProperty('quick')) {
-    apply plugin: 'com.palantir.baseline-checkstyle'
+    // com.palantir.baseline:gradle-baseline-java:4.42.0 (the last version supporting Java 8) pulls
+    // in an old version of the checkstyle(9.1), which has this OutOfMemory bug https://github.com/checkstyle/checkstyle/issues/10934
+    // So, replace com.palantir.baseline-checkstyle plugin usage with gradle checkstyle plugin

Review Comment:
   `So, replace "com.palantir.baseline-checkstyle" with "checkstyle" 9.3 (the latest java 8 supported version) which contains a fix`



-- 
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-unsubscribe@iceberg.apache.org

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] szehon-ho commented on pull request #7321: Build: Fix flaky checkstyle issue

Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #7321:
URL: https://github.com/apache/iceberg/pull/7321#issuecomment-1507264618

   Merged, let's see if this fixes it, thanks @ajantha-bhat for investigation, and @nastra for review


-- 
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-unsubscribe@iceberg.apache.org

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] ajantha-bhat commented on pull request #7321: Build: Fix flaky checkstyle issue

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #7321:
URL: https://github.com/apache/iceberg/pull/7321#issuecomment-1504956204

   PR is ready for review. 


-- 
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-unsubscribe@iceberg.apache.org

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] szehon-ho merged pull request #7321: Build: Fix flaky checkstyle issue

Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho merged PR #7321:
URL: https://github.com/apache/iceberg/pull/7321


-- 
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-unsubscribe@iceberg.apache.org

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] ajantha-bhat commented on pull request #7321: Build: Fix flaky checkstyle failure

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #7321:
URL: https://github.com/apache/iceberg/pull/7321#issuecomment-1503759978

   Oops. This was perfectly compiled locally. But failed in the CI build. 
   probably might have to do with `configDirectory = file("${rootDir}/.baseline/checkstyle")`
   Let me continue the analysis tomorrow. 


-- 
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-unsubscribe@iceberg.apache.org

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] nastra commented on a diff in pull request #7321: Build: Fix flaky checkstyle issue

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7321:
URL: https://github.com/apache/iceberg/pull/7321#discussion_r1163855751


##########
flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/DataGenerators.java:
##########
@@ -124,10 +125,12 @@ private org.apache.avro.Schema fixupAvroSchemaConvertedFromIcebergSchema(
                           LogicalTypes.timeMillis()
                               .addToSchema(
                                   org.apache.avro.Schema.create(org.apache.avro.Schema.Type.INT));
-                      field = new org.apache.avro.Schema.Field("time_field", fieldSchema);
+                      updatedField = new org.apache.avro.Schema.Field("time_field", fieldSchema);
+                    } else {

Review Comment:
   maybe init `updatedField = field` at the beginning rather than having an `else`



-- 
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-unsubscribe@iceberg.apache.org

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