[GitHub] [hadoop] amahussein commented on pull request #2122: [HADOOP-17109] Replace Guava base64Url and base64 with Java8+ base64

[GitBox <gi...@apache.org>]

amahussein commented on pull request #2122:
URL: https://github.com/apache/hadoop/pull/2122#issuecomment-654298701


   Thanks @dengliming for the patch.
   
   - [ ] Can you please list the failing test units? Just to confirm that we went through all of them and enumerating the ones are flaky and the ones could be introduced by our changes.
   
   - [ ] Can you please fix the checkstyle warnings?
   
   - [ ] I see that there are many base64 implementations used throughout the code.I think in that case we change the Jira title and description to reflect the fact that we are also replace `apache common base`.
   
   - [ ] Just to make sure that we have the exact behavior as before, Do you know what are the differences between each of them? Also, any idea about performance between guava, apache common, and java.util?
   
   - `com.google.common.io.BaseEncoding#base64`
   - `com.google.common.io.BaseEncoding#base64Url`
   - `org.apache.commons.codec.binary.Base64`;
   
   
   - [ ] Can you please add the two classes to the illegal imports in checkstyle.xml? It is already done in HADOOP-17111. It should be something like that:
   
   ```
   diff --git hadoop-build-tools/src/main/resources/checkstyle/checkstyle.xml hadoop-build-tools/src/main/resources/checkstyle/checkstyle.xml
   index 8f3d3f13824..54a59437380 100644
   --- hadoop-build-tools/src/main/resources/checkstyle/checkstyle.xml
   +++ hadoop-build-tools/src/main/resources/checkstyle/checkstyle.xml
   @@ -119,7 +119,12 @@
    
            <!-- Checks for imports                              -->
            <!-- See http://checkstyle.sf.net/config_import.html -->
   -        <module name="IllegalImport"/> <!-- defaults to sun.* packages -->
   +        <module name="IllegalImport">
   +          <property name="regexp" value="true"/>
   +          <property name="illegalPkgs" value="^sun\.[^.]+"/>
   +          <property name="illegalClasses"
   +            value="^com\.google\.common\.io\.(BaseEncoding), org.apache.commons.codec.binary.Base64"/>
   +        </module>
            <module name="RedundantImport"/>
            <module name="UnusedImports"/>
   ```


----------------------------------------------------------------
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: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org