You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by GitBox <gi...@apache.org> on 2022/11/08 01:05:47 UTC

[GitHub] [samza] ajothomas opened a new pull request, #1640: SAMZA-2741: [Pipeline Drain] Fix ApplicationUtil.isHighLevelApiJob to work for anonymous and lambda `SamzaApplication` classes

ajothomas opened a new pull request, #1640:
URL: https://github.com/apache/samza/pull/1640

   # Summary
   `ApplicationUtil.isHighLevelApiJob` doesn't work for cases where `SamzaApplication` could have been created as an anonymous class and lambdas
   
   # Cause
   Anonymous class names and lambdas classes cannot be created using `Class forName()`. As a result, the current logic currently assumes that the class name was incorrect. 
   
   # Fix 
   Introduce an internal config `app.api.type` and we set the config using `AppDescriptor` in `JobPlanner`.
   
   # Test
   - Unit test for `ApplicationUtil.isHighLevelApiJob`
   - Tested with a sample application


-- 
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: commits-unsubscribe@samza.apache.org

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

[GitHub] [samza] mynameborat commented on a diff in pull request #1640: SAMZA-2741: [Pipeline Drain] Fix ApplicationUtil.isHighLevelApiJob to work for anonymous and lambda `SamzaApplication` classes

Posted by GitBox <gi...@apache.org>.
mynameborat commented on code in PR #1640:
URL: https://github.com/apache/samza/pull/1640#discussion_r1016065159


##########
samza-core/src/main/java/org/apache/samza/container/RunLoopFactory.java:
##########
@@ -69,6 +69,8 @@ public static Runnable createRunLoop(scala.collection.immutable.Map<TaskName, Ru
 
     log.info("Got current run Id: {}.", runId);
 
+    log.info("Got isHighLevelApiJob flag: {}.", isHighLevelApiJob);

Review Comment:
   Nit: replace `Got isHighLevelApiJob flag: {}` to something meaningful instead of exposing the field/accessor name.



##########
samza-core/src/main/java/org/apache/samza/config/ApplicationConfig.java:
##########
@@ -46,6 +46,11 @@ public enum ApplicationMode {
     BATCH
   }
 
+  public enum ApplicationApiType {
+    LEGACY,
+    LOW_LEVEL,
+    HIGH_LEVEL
+  }

Review Comment:
   Please introduce this `enum` in samza-api module instead?



-- 
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: commits-unsubscribe@samza.apache.org

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

[GitHub] [samza] mynameborat commented on a diff in pull request #1640: SAMZA-2741: [Pipeline Drain] Fix ApplicationUtil.isHighLevelApiJob to work for anonymous and lambda `SamzaApplication` classes

Posted by GitBox <gi...@apache.org>.
mynameborat commented on code in PR #1640:
URL: https://github.com/apache/samza/pull/1640#discussion_r1017221725


##########
samza-core/src/main/java/org/apache/samza/config/ApplicationConfig.java:
##########
@@ -46,6 +46,11 @@ public enum ApplicationMode {
     BATCH
   }
 
+  public enum ApplicationApiType {
+    LEGACY,
+    LOW_LEVEL,
+    HIGH_LEVEL
+  }

Review Comment:
   The reason to do so even if its internally used is to make sure any changes to this enum should be treated as API changes and would need to be treated as backward compatible as this will be used in the coordinator stream configuration which are persisted and read across different versions of samza.
   
   



-- 
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: commits-unsubscribe@samza.apache.org

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

[GitHub] [samza] xinyuiscool merged pull request #1640: SAMZA-2741: [Pipeline Drain] Fix ApplicationUtil.isHighLevelApiJob to work for anonymous and lambda `SamzaApplication` classes

Posted by GitBox <gi...@apache.org>.
xinyuiscool merged PR #1640:
URL: https://github.com/apache/samza/pull/1640


-- 
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: commits-unsubscribe@samza.apache.org

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