date:20240401

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

2024-04-01 Thread via GitHub



tez-yetus commented on PR #341:
URL: https://github.com/apache/tez/pull/341#issuecomment-2030729876

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |   0m  9s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files 
found.  |
   | +0 :ok: |  prototool  |   0m  0s |  prototool 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 
5 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   6m  2s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   9m 16s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 14s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   1m 17s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  checkstyle  |   1m 28s |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 23s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 11s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +0 :ok: |  spotbugs  |   0m 50s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m  1s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m  7s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   0m 53s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 50s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  cc  |   0m 50s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 50s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 49s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  cc  |   0m 49s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 49s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  8s |  The patch passed checkstyle 
in tez-api  |
   | +1 :green_heart: |  checkstyle  |   0m  5s |  The patch passed checkstyle 
in tez-runtime-internals  |
   | +1 :green_heart: |  checkstyle  |   0m  8s |  tez-mapreduce: The patch 
generated 0 new + 43 unchanged - 2 fixed = 43 total (was 45)  |
   | +1 :green_heart: |  checkstyle  |   0m 10s |  The patch passed checkstyle 
in tez-dag  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 40s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  findbugs  |   2m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 57s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 29s |  tez-runtime-internals in the patch 
passed.  |
   | +1 :green_heart: |  unit  |   1m  0s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  unit  |   4m 13s |  tez-dag in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  41m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-341/6/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/341 |
   | JIRA Issue | TEZ-4548 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile cc prototool |
   | uname | Linux a58b56a303df 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / 34bb628e3 |
   | Default Java | Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-341/6/testReport/ |
   | Max. process+thread count | 694 (vs. ulimit of 5500) |
   | modules | C: tez-api tez-runtime-internals tez-mapreduce tez-dag U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-341/6/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

2024-04-01 Thread via GitHub



abstractdog commented on code in PR #341:
URL: https://github.com/apache/tez/pull/341#discussion_r1546922261


##
tez-api/src/main/java/org/apache/tez/runtime/api/events/InputDataInformationEvent.java:
##
@@ -79,6 +79,12 @@ public static InputDataInformationEvent 
createWithObjectPayload(int srcIndex,
 return new InputDataInformationEvent(srcIndex, userPayloadDeserialized, 
null);
   }
 
+  public static InputDataInformationEvent createWithSerializedPath(int 
srcIndex, String serializedPath) {
+InputDataInformationEvent event = new InputDataInformationEvent(srcIndex, 
null);
+event.serializedPath = serializedPath;

Review Comment:
   this method makes it impossible for users to call constructors in an invalid 
way (like: adding payload and serialized path at the same time, which should 
not happen), also this has a method name that implies the expected behavior
   what constructor are you suggesting that has the same advantages?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

2024-04-01 Thread via GitHub



abstractdog commented on code in PR #341:
URL: https://github.com/apache/tez/pull/341#discussion_r1546922261


##
tez-api/src/main/java/org/apache/tez/runtime/api/events/InputDataInformationEvent.java:
##
@@ -79,6 +79,12 @@ public static InputDataInformationEvent 
createWithObjectPayload(int srcIndex,
 return new InputDataInformationEvent(srcIndex, userPayloadDeserialized, 
null);
   }
 
+  public static InputDataInformationEvent createWithSerializedPath(int 
srcIndex, String serializedPath) {
+InputDataInformationEvent event = new InputDataInformationEvent(srcIndex, 
null);
+event.serializedPath = serializedPath;

Review Comment:
   this method makes it impossible for users to call constructors in an invalid 
way (like: adding payload and serialized path at the same time, which should 
not happen), also with having a method name that implies this behavior
   what constructor are you suggesting that has the same advantages?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

2024-04-01 Thread via GitHub



aturoczy commented on code in PR #341:
URL: https://github.com/apache/tez/pull/341#discussion_r1546922508


##
tez-api/src/main/java/org/apache/tez/runtime/api/events/InputDataInformationEvent.java:
##
@@ -103,6 +113,7 @@ public Object getDeserializedUserPayload() {
   public String toString() {
 return "InputDataInformationEvent [sourceIndex=" + sourceIndex + ", 
targetIndex="
 + targetIndex + ", serializedUserPayloadExists=" + (userPayload != 
null)
-+ ", deserializedUserPayloadExists=" + (userPayloadObject != null) + 
"]";
-  } 
++ ", deserializedUserPayloadExists=" + (userPayloadObject != null)
++ ", serializedPath=" + serializedPath + "]";

Review Comment:
   I like it :) I know it add a bit complexity into a simple toString() but it 
is more elegant imho



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

2024-04-01 Thread via GitHub



abstractdog commented on code in PR #341:
URL: https://github.com/apache/tez/pull/341#discussion_r1546922261


##
tez-api/src/main/java/org/apache/tez/runtime/api/events/InputDataInformationEvent.java:
##
@@ -79,6 +79,12 @@ public static InputDataInformationEvent 
createWithObjectPayload(int srcIndex,
 return new InputDataInformationEvent(srcIndex, userPayloadDeserialized, 
null);
   }
 
+  public static InputDataInformationEvent createWithSerializedPath(int 
srcIndex, String serializedPath) {
+InputDataInformationEvent event = new InputDataInformationEvent(srcIndex, 
null);
+event.serializedPath = serializedPath;

Review Comment:
   this method makes it impossible for users to call constructors in an invalid 
way (like: adding payload and serialized path at the same time), also with 
having a method name that implies this behavior, what constructor are you 
suggesting that has the same advantages?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

2024-04-01 Thread via GitHub



abstractdog commented on code in PR #341:
URL: https://github.com/apache/tez/pull/341#discussion_r1546918816


##
tez-api/src/main/java/org/apache/tez/runtime/api/events/InputDataInformationEvent.java:
##
@@ -103,6 +113,7 @@ public Object getDeserializedUserPayload() {
   public String toString() {
 return "InputDataInformationEvent [sourceIndex=" + sourceIndex + ", 
targetIndex="
 + targetIndex + ", serializedUserPayloadExists=" + (userPayload != 
null)
-+ ", deserializedUserPayloadExists=" + (userPayloadObject != null) + 
"]";
-  } 
++ ", deserializedUserPayloadExists=" + (userPayloadObject != null)
++ ", serializedPath=" + serializedPath + "]";

Review Comment:
   I can add a null check like:
   ```
+ serializedPath != null ? (", serializedPath=" + serializedPath) : "" + 
"]";
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

2024-04-01 Thread via GitHub



aturoczy commented on code in PR #341:
URL: https://github.com/apache/tez/pull/341#discussion_r1546906996


##
tez-api/src/main/java/org/apache/tez/runtime/api/events/InputDataInformationEvent.java:
##
@@ -103,6 +113,7 @@ public Object getDeserializedUserPayload() {
   public String toString() {
 return "InputDataInformationEvent [sourceIndex=" + sourceIndex + ", 
targetIndex="
 + targetIndex + ", serializedUserPayloadExists=" + (userPayload != 
null)
-+ ", deserializedUserPayloadExists=" + (userPayloadObject != null) + 
"]";
-  } 
++ ", deserializedUserPayloadExists=" + (userPayloadObject != null)
++ ", serializedPath=" + serializedPath + "]";

Review Comment:
   If serializedPath is null then it would look a bit odd. I like to handle 
this type of scenario. But this is just styling :) 



##
tez-api/src/main/java/org/apache/tez/runtime/api/events/InputDataInformationEvent.java:
##
@@ -79,6 +79,12 @@ public static InputDataInformationEvent 
createWithObjectPayload(int srcIndex,
 return new InputDataInformationEvent(srcIndex, userPayloadDeserialized, 
null);
   }
 
+  public static InputDataInformationEvent createWithSerializedPath(int 
srcIndex, String serializedPath) {
+InputDataInformationEvent event = new InputDataInformationEvent(srcIndex, 
null);
+event.serializedPath = serializedPath;

Review Comment:
   Won't just be easier to add a 3rd constructor for the 
InputDataInformationEvent? To set a property is fine, just not fail safe.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-01 Thread via GitHub



tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2030457385

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |  26m 48s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files 
found.  |
   | +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 
1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 33s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  10m 15s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 54s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   1m 44s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  checkstyle  |   2m  2s |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 41s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 29s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +0 :ok: |  spotbugs  |   1m 22s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 47s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 16s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   1m 16s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  javac  |   1m  5s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 13s |  tez-api: The patch generated 1 
new + 16 unchanged - 0 fixed = 17 total (was 16)  |
   | -0 :warning: |  checkstyle  |   0m 18s |  tez-mapreduce: The patch 
generated 4 new + 368 unchanged - 0 fixed = 372 total (was 368)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  findbugs  |   3m  4s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 15s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 21s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  unit  |   5m  0s |  tez-dag in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  75m 27s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.45 ServerAPI=1.45 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux 143e09ab5003 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / 34bb628e3 |
   | Default Java | Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/artifact/out/diff-checkstyle-tez-api.txt
 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/testReport/ |
   | Max. process+thread count | 403 (vs. ulimit of 5500) |
   | modules | C: tez-api tez-mapreduce tez-dag U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.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

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

Re: [PR] TEZ-4548: InputDataInformationEvent to be read from serialized payload from filesystem [tez]

Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

8 matches

Site Navigation

Mail list logo

Footer information