You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/04/21 23:44:39 UTC

[GitHub] [hadoop] bbeaudreault opened a new pull request, #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

bbeaudreault opened a new pull request, #4215:
URL: https://github.com/apache/hadoop/pull/4215

   ### Description of PR
   Very simple one-line change that allows users to optionally call addName for key/value classes that may not inherit Writable (instead relying on serialization). 
   
   ### How was this patch tested?
   Added two tests:
   1. Simple unit test in TestWritableName
   2. More of an integration test in TestSequenceFile which verifies that it works to read and write with shimmed serializable classes.
   
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] omalley commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "omalley (via GitHub)" <gi...@apache.org>.
omalley commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1433725058

   I think branch-3 is mostly abandoned. I committed it to branch-3.3. It can be committed in my opinion to branch-3.3.5, but I'll leave that call up to the release manager.


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1105975229

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |  13m 26s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  41m  0s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  24m 52s |  |  trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  24m 17s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m 39s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m  7s |  |  trunk passed  |
   | -1 :x: |  javadoc  |   1m 49s | [/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/1/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) |  hadoop-common in trunk failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.  |
   | +1 :green_heart: |  javadoc  |   2m  1s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m 22s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  28m  6s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m 13s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  23m 45s |  |  the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  23m 45s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  21m 55s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  21m 55s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   1m 32s | [/results-checkstyle-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/1/artifact/out/results-checkstyle-hadoop-common-project_hadoop-common.txt) |  hadoop-common-project/hadoop-common: The patch generated 5 new + 68 unchanged - 0 fixed = 73 total (was 68)  |
   | +1 :green_heart: |  mvnsite  |   2m  4s |  |  the patch passed  |
   | -1 :x: |  javadoc  |   1m 43s | [/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/1/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) |  hadoop-common in the patch failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.  |
   | +1 :green_heart: |  javadoc  |   2m 11s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m 10s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m 40s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 31s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 33s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 244m 51s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux b760f89553c9 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / eee1ea23239a08e5c995f77b31e1cf5c981d54e4 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/1/testReport/ |
   | Max. process+thread count | 1266 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/1/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] omalley commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "omalley (via GitHub)" <gi...@apache.org>.
omalley commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1107882426


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -79,20 +79,42 @@ public static synchronized String getName(Class<?> writableClass) {
     return writableClass.getName();
   }
 
+  /**
+   * Return the class for a name. Requires the class for name to extend Writable.
+   * See {@link #getClass(String, Configuration, boolean)} if class doesn't extend Writable.
+   * Default is {@link Class#forName(String)}.
+   *
+   * @param name input name.
+   * @param conf input configuration.
+   * @return class for a name.
+   * @throws IOException raised on errors performing I/O.
+   */
+  public static synchronized Class<?> getClass(String name, Configuration conf)
+      throws IOException {
+    return getClass(name, conf, true);
+  }
+
   /**
    * Return the class for a name.
    * Default is {@link Class#forName(String)}.
    *
    * @param name input name.
    * @param conf input configuration.
+   * @param requireWritable if true, require the class for name to extend Writable
    * @return class for a name.
    * @throws IOException raised on errors performing I/O.
    */
-  public static synchronized Class<?> getClass(String name, Configuration conf
-                                            ) throws IOException {
+  public static synchronized Class<?> getClass(String name, Configuration conf,
+      boolean requireWritable) throws IOException {

Review Comment:
   I'd propose that we unconditionally remove the check for Writable in getClass. My thought is:
   * Users can always enforce the constraint later if they want to.
   * All uses of the method with Hadoop's code base don't want to limit the output.
   * The check isn't consistent. (It is only applied for aliases, not natural class names.)
   * Removing the check won't break any callers since they couldn't get non-Writables before.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1106558387


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   thx. the method is there since 2009, that is deemed stable :)



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1107974086


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -79,20 +79,42 @@ public static synchronized String getName(Class<?> writableClass) {
     return writableClass.getName();
   }
 
+  /**
+   * Return the class for a name. Requires the class for name to extend Writable.
+   * See {@link #getClass(String, Configuration, boolean)} if class doesn't extend Writable.
+   * Default is {@link Class#forName(String)}.
+   *
+   * @param name input name.
+   * @param conf input configuration.
+   * @return class for a name.
+   * @throws IOException raised on errors performing I/O.
+   */
+  public static synchronized Class<?> getClass(String name, Configuration conf)
+      throws IOException {
+    return getClass(name, conf, true);
+  }
+
   /**
    * Return the class for a name.
    * Default is {@link Class#forName(String)}.
    *
    * @param name input name.
    * @param conf input configuration.
+   * @param requireWritable if true, require the class for name to extend Writable
    * @return class for a name.
    * @throws IOException raised on errors performing I/O.
    */
-  public static synchronized Class<?> getClass(String name, Configuration conf
-                                            ) throws IOException {
+  public static synchronized Class<?> getClass(String name, Configuration conf,
+      boolean requireWritable) throws IOException {

Review Comment:
   @ayushtkn @omalley -- ok we're back to the original change here. Please give it another look and let me know if you'd like me to make any other changes.
   
   Thanks again



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1430703431

   Just want to clarify the use-case here (as a reminder and for the new reviewers):
   
   - When you write a SequenceFile, the key class and value class are encoded in the format.
   - When you go to read that file later, the SequenceFile.Reader gets those key/value class names from the headers and tries to load the class so it can parse the keys/values. This is where WritableName first comes in, with the default being `Class.forName(keyClass)`.
   - However, some SequenceFiles may be very old and the classes may have been renamed. In this case, the above would throw ClassNotFoundException.
   - This is why `WritableName.addName` exists, which allows you to specify aliases pointing those old class names at whatever the new name is. When `WritableName.getClass` is called it will check to see if an alias was registered by calling addName prior to opening the SequenceFile. If so, it returns that class.
   
   This all worked when all key/value extend Writable, but Hadoop also supports Serialization framework. You can specify `io.serializations` to register serializations, and the SerializationFactory will try finding a serializer for the key or value class. Serializations have an `boolean accept(Class)` method, and one of the registered serializations need to return true for that.
   
   So when the same "old sequence file contains a key or value class that has been renamed" problem happens, if you are using Serialization you are out of luck. By default you'd get a ClassNotFoundException, and if you tried doing WritableName.addName, you'd get ClassCastException.
   
   ----
   
   The simplest fix for that seemed to be in WritableName which appeared to be IA.Private and have no real usages in the repo outside of SequenceFile. A one line change there was attractive. The risk here seems pretty low, at least for how SequenceFile uses this class.
   
   If we have concerns here, there are other possible more involved solutions we could discuss. For example, we could add something in SerializationFactory to add aliases. This would be more involved though because it'd require a slight refactor in SequenceFile and we'd have to make sure that new API worked for any other usages of SerializationFactory.
   
   That's why I chose the simple 1 liner approach, since it solves the problem with simplicity and minimal external impact.


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1107886844

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 39s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  39m  4s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  23m 14s |  |  trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  20m 40s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m 48s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 10s |  |  trunk passed  |
   | -1 :x: |  javadoc  |   1m 51s | [/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/2/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) |  hadoop-common in trunk failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.  |
   | +1 :green_heart: |  javadoc  |   2m 19s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m 10s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  23m 50s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  4s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  23m 44s |  |  the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  23m 44s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  21m  1s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  21m  1s |  |  the patch passed  |
   | -1 :x: |  blanks  |   0m  0s | [/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/2/artifact/out/blanks-eol.txt) |  The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  checkstyle  |   1m 43s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   2m  8s |  |  the patch passed  |
   | -1 :x: |  javadoc  |   1m 29s | [/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/2/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) |  hadoop-common in the patch failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.  |
   | +1 :green_heart: |  javadoc  |   2m  9s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m 14s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m 58s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 28s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 30s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 220m 29s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux e6fb31e223b4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 537dae495cd77d59b89063573396d666672dc47e |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/2/testReport/ |
   | Max. process+thread count | 1266 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/2/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1431288260

   Sounds good, thanks. The one liner is not strict, I was just giving the context so everyone understood the use-case/reasons. 
   
   I'm happy to merge as is if you'd like, but we can also wait a few days to see if any of the people you pinged chime in. 


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1164645318

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 49s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  1s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | -1 :x: |  mvninstall  |  37m 16s | [/branch-mvninstall-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/branch-mvninstall-root.txt) |  root in trunk failed.  |
   | -1 :x: |  compile  |   0m 54s | [/branch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/branch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) |  root in trunk failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.  |
   | -1 :x: |  compile  |   0m 54s | [/branch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/branch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) |  root in trunk failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.  |
   | -0 :warning: |  checkstyle  |   0m 51s | [/buildtool-branch-checkstyle-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/buildtool-branch-checkstyle-hadoop-common-project_hadoop-common.txt) |  The patch fails to run checkstyle in hadoop-common  |
   | -1 :x: |  mvnsite  |   0m 51s | [/branch-mvnsite-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/branch-mvnsite-hadoop-common-project_hadoop-common.txt) |  hadoop-common in trunk failed.  |
   | -1 :x: |  javadoc  |   0m 58s | [/branch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) |  hadoop-common in trunk failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.  |
   | -1 :x: |  javadoc  |   0m 47s | [/branch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) |  hadoop-common in trunk failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.  |
   | -1 :x: |  spotbugs  |   0m 52s | [/branch-spotbugs-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/branch-spotbugs-hadoop-common-project_hadoop-common.txt) |  hadoop-common in trunk failed.  |
   | +1 :green_heart: |  shadedclient  |   6m 30s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 33s | [/patch-mvninstall-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-mvninstall-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch failed.  |
   | -1 :x: |  compile  |   0m 33s | [/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) |  root in the patch failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.  |
   | -1 :x: |  javac  |   0m 33s | [/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) |  root in the patch failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.  |
   | -1 :x: |  compile  |   0m 33s | [/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) |  root in the patch failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.  |
   | -1 :x: |  javac  |   0m 33s | [/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) |  root in the patch failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   0m 30s | [/buildtool-patch-checkstyle-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/buildtool-patch-checkstyle-hadoop-common-project_hadoop-common.txt) |  The patch fails to run checkstyle in hadoop-common  |
   | -1 :x: |  mvnsite  |   0m 32s | [/patch-mvnsite-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-mvnsite-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch failed.  |
   | -1 :x: |  javadoc  |   0m 34s | [/patch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) |  hadoop-common in the patch failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.  |
   | -1 :x: |  javadoc  |   0m 33s | [/patch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) |  hadoop-common in the patch failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.  |
   | -1 :x: |  spotbugs  |   0m 34s | [/patch-spotbugs-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-spotbugs-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch failed.  |
   | +1 :green_heart: |  shadedclient  |   6m 46s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m 32s | [/patch-unit-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch failed.  |
   | +0 :ok: |  asflicense  |   0m 32s |  |  ASF License check generated no output?  |
   |  |   |  57m 48s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 588fe4cc01f4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 52fc693a714ad62aff9bc4b729f2f52200553786 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/testReport/ |
   | Max. process+thread count | 93 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/4/console |
   | versions | git=2.25.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1106084218


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   I was thinking this would be a safe change from a compatibility perspective. People probably shouldn't have code calling this with writables, since it would just fail. I would think the inverse (throwing a new exception) would be more problematic. Is there a documented compatibility guideline for Hadoop?
   
   But yea I can do that if you think it's necessary, just might be nicer without. 



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1107912320


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -79,20 +79,42 @@ public static synchronized String getName(Class<?> writableClass) {
     return writableClass.getName();
   }
 
+  /**
+   * Return the class for a name. Requires the class for name to extend Writable.
+   * See {@link #getClass(String, Configuration, boolean)} if class doesn't extend Writable.
+   * Default is {@link Class#forName(String)}.
+   *
+   * @param name input name.
+   * @param conf input configuration.
+   * @return class for a name.
+   * @throws IOException raised on errors performing I/O.
+   */
+  public static synchronized Class<?> getClass(String name, Configuration conf)
+      throws IOException {
+    return getClass(name, conf, true);
+  }
+
   /**
    * Return the class for a name.
    * Default is {@link Class#forName(String)}.
    *
    * @param name input name.
    * @param conf input configuration.
+   * @param requireWritable if true, require the class for name to extend Writable
    * @return class for a name.
    * @throws IOException raised on errors performing I/O.
    */
-  public static synchronized Class<?> getClass(String name, Configuration conf
-                                            ) throws IOException {
+  public static synchronized Class<?> getClass(String name, Configuration conf,
+      boolean requireWritable) throws IOException {

Review Comment:
   Sounds good, thanks for looking!
   
   @ayushtkn if you're ok with that I can just revert my last commit 



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1164781759

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 44s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  38m 41s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  23m  6s |  |  trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  compile  |  21m  1s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m 17s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 41s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 12s |  |  trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 46s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 44s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  22m 33s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 59s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  23m 18s |  |  the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javac  |  23m 18s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  21m  8s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  21m  8s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  9s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 39s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  4s |  |  the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 37s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m  7s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 35s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m  6s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 209m 44s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 85b4da9d0004 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 52fc693a714ad62aff9bc4b729f2f52200553786 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/5/testReport/ |
   | Max. process+thread count | 1549 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/5/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1433744516

   Thanks Steve and Owen for chimming in and helping over here.
   
   Thanx Bryan for the contribution, One year!!!


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1433548828

   Thanks everyone :)


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1107931995

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 39s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  39m  0s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  23m 12s |  |  trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  20m 37s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m 47s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 11s |  |  trunk passed  |
   | -1 :x: |  javadoc  |   1m 53s | [/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/3/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) |  hadoop-common in trunk failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.  |
   | +1 :green_heart: |  javadoc  |   2m 16s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m 12s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  23m 40s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  4s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m 26s |  |  the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  22m 26s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m 39s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  20m 39s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 40s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   2m  4s |  |  the patch passed  |
   | -1 :x: |  javadoc  |   1m 42s | [/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/3/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) |  hadoop-common in the patch failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.  |
   | +1 :green_heart: |  javadoc  |   2m 16s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m  9s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m 37s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 30s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 35s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 218m 31s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux 63ab0accc8d8 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 796d90102067c71c35dc1eacacd74bb090794f17 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/3/testReport/ |
   | Max. process+thread count | 1266 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/3/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] steveloughran commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1431786735

   blocker with anything related to writable &c are "do you break any existing serializations, subclasses etc". we know that parquet and avro do that and we don't want to maintain the tradition.
   
   @omalley is the person to review the code


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1430704163

   > In general sounds fine to me. Do we need an extra validation that the class `implements Serialization`?
   
   
   So these serializable classes don't need to implement Serializable interface. The only real check we could do is whether [`SerializationFactory.getSerialization(class)`](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/serializer/SerializationFactory.java#L98-L104) returns non-null for the class. Not sure we want to do that. SequenceFile already validates that the class has a serialization [here](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SequenceFile.java#L2132-L2142).
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1106558387


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   thx. the method is there since 2009, that is deemed stable :)
   In general there is a line as well in the compat
   ```
   the expectation that an interface should “eventually” stabilize and be promoted to Stable
   ```
   but we aren't changing and the hadoop utils I feel are widely used, and these annotations just help to dodge the complains from downstream when their code breaks, "Your fault, Hadoop never said you can use this"



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1186281286

   @jojochuang thabka again for looking at this. Do you think you could merge given the above test results?


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1226605432

   @jojochuang Any chance we can merge this?


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1433552876

   Is it possible to backport to branch-3 and/or branch-3.3? Not sure how that works in the hadoop project. I can submit follow-up PRs, but I imagine this will apply cleanly with cherry-pick.


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1106403146


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   Sounds good @ayushtkn. This class is annotated:
   
   ```
   @InterfaceAudience.Private
   @InterfaceStability.Evolving
   ```
   
   So I thought we had more leeway (though i'm more used to hbase project).
   
   I just pushed the change you requested.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1106063559


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   It is a public method and you are changing the behaviour of this. Earlier for which classes it used to return an Exception.
   Now it will be returning some result.
   
   I would suggest add a new method, which can take an argument based on which it can do .asSubclass(Writable.class) or not, And you can refactor whatever is inside this method to that new method with an extra flag, and call it from the existing method with false or so. 
   
   Moreover Slack has limited audience, the dev lists have more, so good to give it a try if none of the options work



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   It is a public method and you are changing the behaviour of this. Earlier for which classes it used to return an Exception.
   Now it will be returning some result.
   
   I would suggest add a new method, which can take an argument based on which it can do .asSubclass(Writable.class) or not, And you can refactor whatever is inside this method to that new method with an extra flag, and call it from the existing method with false or so. 
   



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1428481396

   @jojochuang any chance we can merge this?


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1107967867


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -79,20 +79,42 @@ public static synchronized String getName(Class<?> writableClass) {
     return writableClass.getName();
   }
 
+  /**
+   * Return the class for a name. Requires the class for name to extend Writable.
+   * See {@link #getClass(String, Configuration, boolean)} if class doesn't extend Writable.
+   * Default is {@link Class#forName(String)}.
+   *
+   * @param name input name.
+   * @param conf input configuration.
+   * @return class for a name.
+   * @throws IOException raised on errors performing I/O.
+   */
+  public static synchronized Class<?> getClass(String name, Configuration conf)
+      throws IOException {
+    return getClass(name, conf, true);
+  }
+
   /**
    * Return the class for a name.
    * Default is {@link Class#forName(String)}.
    *
    * @param name input name.
    * @param conf input configuration.
+   * @param requireWritable if true, require the class for name to extend Writable
    * @return class for a name.
    * @throws IOException raised on errors performing I/O.
    */
-  public static synchronized Class<?> getClass(String name, Configuration conf
-                                            ) throws IOException {
+  public static synchronized Class<?> getClass(String name, Configuration conf,
+      boolean requireWritable) throws IOException {

Review Comment:
   Go ahead



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1106063559


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   It is a public method and you are changing the behaviour of this. Earlier for which classes it used to return an Exception.
   Now it will be returning some result.
   
   I would suggest add a new method, which can take an argument based on which it can do .asSubclass(Writable.class) or not, And you can refactor whatever is inside this method to that new method with an extra flag, and call it from the existing method with false or so.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#discussion_r1106140456


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableName.java:
##########
@@ -92,7 +92,7 @@ public static synchronized Class<?> getClass(String name, Configuration conf
                                             ) throws IOException {
     Class<?> writableClass = NAME_TO_CLASS.get(name);
     if (writableClass != null)
-      return writableClass.asSubclass(Writable.class);
+      return writableClass;

Review Comment:
   That is what we expect, nobody should be expecting an exception. But this method used to throw an exception earlier and we should ensure unless it is a bug in the code, or the last option we should let it stay as is. Such changes looks safe on hadoop side, but when a downstream project tries to upgrade and if it is using such methods, it creates issues and blocks the way for them.(Have seen that in the past with Hive/Tez)
   
   So, I would say if we can avoid changing the existing behaviour of a public method, we should do as a best effort



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1430603452

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 43s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  43m 48s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  23m  4s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  compile  |  20m 29s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   1m 13s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 41s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 16s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   2m 42s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  25m 29s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  8s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m 26s |  |  the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javac  |  22m 26s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m 38s |  |  the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  javac  |  20m 38s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  |  hadoop-common-project/hadoop-common: The patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318)  |
   | +1 :green_heart: |  mvnsite  |   1m 41s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  |  the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  |  the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   2m 39s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  25m 39s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 25s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m  2s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 218m 28s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/7/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux f61723f97b48 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 33d6d4c9f38033bb1e871723ddc300d0bd4edeb7 |
   | Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/7/testReport/ |
   | Max. process+thread count | 1997 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/7/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1164971314

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 39s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  37m 50s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  23m 15s |  |  trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  compile  |  20m 39s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m 48s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 14s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 36s |  |  trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 22s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m 11s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  23m 34s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  7s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m 23s |  |  the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javac  |  22m 23s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m 41s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  20m 41s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 39s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   2m  9s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 38s |  |  the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 20s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   3m  8s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m 27s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 47s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 35s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 215m 49s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/6/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 0cb34f79ee1f 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / adf3de0112c97cb637b72c19aacddb0662bdee4d |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/6/testReport/ |
   | Max. process+thread count | 2466 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/6/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1171129088

   @jojochuang thank you very much for taking a look. Are you sure you tested correctly?
   
   Adding back the `.asSubclass(Writable.class)` in WritableName and re-running the tests, I get the following 2 expected failures:
   
   ```
   [ERROR] Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.521 s <<< FAILURE! - in org.apache.hadoop.io.TestWritableName
   [ERROR] testAddNameSerializable(org.apache.hadoop.io.TestWritableName)  Time elapsed: 0.133 s  <<< ERROR!
   java.lang.ClassCastException: class org.apache.hadoop.io.TestWritableName$SimpleSerializable
   	at java.base/java.lang.Class.asSubclass(Class.java:3640)
   	at org.apache.hadoop.io.WritableName.getClass(WritableName.java:95)
   	at org.apache.hadoop.io.TestWritableName.testAddNameSerializable(TestWritableName.java:139)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
   	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
   	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
   	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
   	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
   	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
   	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
   	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
   	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
   	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
   	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
   	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
   	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
   	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
   
   [INFO] Running org.apache.hadoop.io.TestSequenceFile
   [ERROR] Tests run: 11, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 15.45 s <<< FAILURE! - in org.apache.hadoop.io.TestSequenceFile
   [ERROR] testSerializationUsingWritableNameAlias(org.apache.hadoop.io.TestSequenceFile)  Time elapsed: 0.035 s  <<< ERROR!
   java.lang.ClassCastException: class org.apache.hadoop.io.TestSequenceFile$AnotherSimpleSerializable
   	at java.base/java.lang.Class.asSubclass(Class.java:3640)
   	at org.apache.hadoop.io.WritableName.getClass(WritableName.java:95)
   	at org.apache.hadoop.io.SequenceFile$Reader.getKeyClass(SequenceFile.java:2199)
   	at org.apache.hadoop.io.SequenceFile$Reader.init(SequenceFile.java:2133)
   	at org.apache.hadoop.io.SequenceFile$Reader.initialize(SequenceFile.java:1982)
   	at org.apache.hadoop.io.SequenceFile$Reader.<init>(SequenceFile.java:1931)
   	at org.apache.hadoop.io.TestSequenceFile.testSerializationUsingWritableNameAlias(TestSequenceFile.java:795)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
   	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
   	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
   	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
   	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
   	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
   	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
   	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
   	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
   	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
   	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
   	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
   	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
   	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
   
   [INFO]
   [INFO] Results:
   [INFO]
   [ERROR] Errors:
   [ERROR]   TestSequenceFile.testSerializationUsingWritableNameAlias:795 » ClassCast class...
   [ERROR]   TestWritableName.testAddNameSerializable:139 » ClassCast class org.apache.hado...
   [INFO]
   [ERROR] Tests run: 16, Failures: 0, Errors: 2, Skipped: 0
   ```


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] bbeaudreault commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1430056678

   Thanks for looking and for the suggestions! Just wanted to ask one question about your review comment before I make the change. 


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] omalley merged pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "omalley (via GitHub)" <gi...@apache.org>.
omalley merged PR #4215:
URL: https://github.com/apache/hadoop/pull/4215


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] hadoop-yetus commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1432636826

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 40s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  44m 32s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  22m 58s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  compile  |  20m 28s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   1m 16s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 44s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 17s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   2m 41s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  25m 26s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  5s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m 28s |  |  the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javac  |  22m 28s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m 25s |  |  the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  javac  |  20m 24s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  8s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   2m 17s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  8s |  |  the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  |  the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   2m 39s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  25m 42s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 18s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m  2s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 219m 34s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/8/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4215 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 1bf9a9f6b9a6 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / b28373311832847fb73deb1b47e4dea79748b812 |
   | Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/8/testReport/ |
   | Max. process+thread count | 1641 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4215/8/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


[GitHub] [hadoop] ayushtkn commented on pull request #4215: HADOOP-18215. Enhance WritableName to be able to return aliases for classes that use serializers

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #4215:
URL: https://github.com/apache/hadoop/pull/4215#issuecomment-1430812485

   To me it sounds ok, regarding the one liner, if that is a strickt ask and if other people are convinced I am ok with that as well. But it isn't something where I am the only one approving it. I will play it safe, Utility classes does get used a lot, Even FileUtils, Utils are all annotated Evolving, but if you break it, you will break a lot of stuff. Something which is there more than 10 years....
   
   The codes being impacted are around 2008-09, I am not sure who has direct involvement during development, like why it was done like that or so.
   
   @steveloughran / @goiri / @vinayakumarb anyone any context or feedback around this?


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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