You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/04/08 14:18:13 UTC

[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #788: HDDS-3357. Add check for import from shaded package

adoroszlai opened a new pull request #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788
 
 
   ## What changes were proposed in this pull request?
   
   Add a checkstyle rule to reject import from shaded packages.  This helps avoid accidentally using shaded transitive dependencies instead of direct, unshaded dependencies, eg. `org.apache.curator.shaded.com.google.common.collect.ImmutableSet` instead of `com.google.common.collect.ImmutableSet`.
   
   Also skip frontend install/compile for Recon (as checkstyle only applies to Java code) similarly to ac93176a0.
   
   https://issues.apache.org/jira/browse/HDDS-3357
   
   ## How was this patch tested?
   
   Temporarily added an import from a `sun.*` package, verified that it is flagged (along with the only current import from shaded package):
   
   ```
   $ hadoop-ozone/dev-support/checks/checkstyle.sh
   ...
   hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java
    21: Illegal import - sun.security.ec.ECKeyFactory.
   hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSetDiskChecks.java
    38: Illegal import - org.apache.curator.shaded.com.google.common.collect.ImmutableSet.
   $ echo $?
   1
   ```
   
   Failed check due to import from shaded package:
   https://github.com/adoroszlai/hadoop-ozone/runs/570614153
   
   Successful check after removing offender:
   https://github.com/adoroszlai/hadoop-ozone/runs/570825882

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] lokeshj1703 commented on issue #788: HDDS-3357. Add check for import from shaded package

Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on issue #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788#issuecomment-613948734
 
 
   @adoroszlai Thanks for clarifying! I am +1 on the PR.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] adoroszlai commented on issue #788: HDDS-3357. Add check for import from shaded package

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on issue #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788#issuecomment-614487998
 
 
   Thanks @fapifta and @lokeshj1703 for the review and @vivekratnavel for review and merge.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] adoroszlai commented on issue #788: HDDS-3357. Add check for import from shaded package

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on issue #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788#issuecomment-613431896
 
 
   Thanks @lokeshj1703 for the review.
   
   > Why do we need -Dskip.yarn and -Dskip.installyarn in MAVEN_OPTIONS?
   
   `yarn` in this case refers to the [Javascript package manager](https://yarnpkg.com/) by this name.  These flags save some time during the build, since Checkstyle does not care about Javascript code.  #275 added similar feature in the Findbugs script, which is also Java-specific.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] adoroszlai commented on issue #788: HDDS-3357. Add check for import from shaded package

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on issue #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788#issuecomment-610989183
 
 
   @fapifta 70df72dc58d5d3067f6f45a8233ff6683dc3a1f5 is your change, it's included here only to make the check pass.  If you create a PR with it, we can merge that first.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] fapifta commented on issue #788: HDDS-3357. Add check for import from shaded package

Posted by GitBox <gi...@apache.org>.
fapifta commented on issue #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788#issuecomment-613478700
 
 
   Hmm, great improvement, the changes seem to be good, and I support committing them (non-binding +1), I have added the mentioned changeset a bit late... it is under PR #818 currently being checked by automated checks.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] vivekratnavel commented on issue #788: HDDS-3357. Add check for import from shaded package

Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on issue #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788#issuecomment-614410865
 
 
   @adoroszlai Thanks for working on this! Merged it.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] vivekratnavel merged pull request #788: HDDS-3357. Add check for import from shaded package

Posted by GitBox <gi...@apache.org>.
vivekratnavel merged pull request #788: HDDS-3357. Add check for import from shaded package
URL: https://github.com/apache/hadoop-ozone/pull/788
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org