You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/25 03:21:51 UTC

[GitHub] [beam] boyuanzz opened a new pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload

boyuanzz opened a new pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload
URL: https://github.com/apache/beam/pull/11216
 
 
   **Please** add a meaningful description for your change here
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
   --- | --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604673796
 
 
   Run Python2_PVR_Flink PreCommit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload

Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload
URL: https://github.com/apache/beam/pull/11216#discussion_r397942726
 
 

 ##########
 File path: sdks/python/apache_beam/transforms/userstate.py
 ##########
 @@ -141,6 +143,13 @@ def __init__(self, name, time_domain):
   def __repr__(self):
     return '%s(%s)' % (self.__class__.__name__, self.name)
 
+  def to_timer_family_runner_api(self, context):
 
 Review comment:
   shouldn't this method replace `to_runner_api`?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604673442
 
 
   Run JavaPortabilityApi PreCommit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604688875
 
 
   Run Python PreCommit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload

Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload
URL: https://github.com/apache/beam/pull/11216#discussion_r397943829
 
 

 ##########
 File path: sdks/python/apache_beam/transforms/userstate.py
 ##########
 @@ -149,6 +158,26 @@ def to_runner_api(self, context):
             coders._TimerCoder(coders.SingletonCoder(None))))
 
 
+# TODO(BEAM-9602): Provide support of dynamic timer
 
 Review comment:
   ```suggestion
   # TODO(BEAM-9602): Provide support for dynamic timer
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] suztomo edited a comment on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
suztomo edited a comment on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-606158291
 
 
   @boyuanzz @lukecwik  Run Dataflow ValidatesRunner (beam_PostCommit_Java_ValidatesRunner_Dataflow) started failing after this PR:
   https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/4719/
   
   History:
   
   <img width="690" alt="Screen Shot 2020-03-30 at 14 07 08" src="https://user-images.githubusercontent.com/28604/77946079-c3421880-728f-11ea-9b0a-e5b55b078db8.png">
   
   
   ```
   00:27:33 > Task :runners:google-cloud-dataflow-java:validatesRunnerLegacyWorkerTest
   00:28:48 
   00:28:48 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerWithMultipleTimerFamilyUnbounded FAILED
   00:28:48     java.lang.RuntimeException at ParDoTest.java:4770
   00:28:48         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4770
   00:42:57 
   00:42:57 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerFamilyEventTimeUnbounded FAILED
   00:42:57     java.lang.RuntimeException at ParDoTest.java:4708
   00:42:57         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4708
   ```
   
   I appreciate if you can check whether the failure is related to this PR.
   
   Created an issue: https://issues.apache.org/jira/browse/BEAM-9636

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] suztomo edited a comment on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
suztomo edited a comment on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-606158291
 
 
   @boyuanzz @lukecwik  beam_PostCommit_Java_ValidatesRunner_Dataflow started failing after this PR:
   https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/4719/
   
   History:
   
   <img width="690" alt="Screen Shot 2020-03-30 at 14 07 08" src="https://user-images.githubusercontent.com/28604/77946079-c3421880-728f-11ea-9b0a-e5b55b078db8.png">
   
   
   ```
   00:27:33 > Task :runners:google-cloud-dataflow-java:validatesRunnerLegacyWorkerTest
   00:28:48 
   00:28:48 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerWithMultipleTimerFamilyUnbounded FAILED
   00:28:48     java.lang.RuntimeException at ParDoTest.java:4770
   00:28:48         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4770
   00:42:57 
   00:42:57 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerFamilyEventTimeUnbounded FAILED
   00:42:57     java.lang.RuntimeException at ParDoTest.java:4708
   00:42:57         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4708
   ```
   
   I appreciate if you can check whether the failure is related to this PR.
   
   Created an issue: https://issues.apache.org/jira/browse/BEAM-9636

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload

Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload
URL: https://github.com/apache/beam/pull/11216#discussion_r397940099
 
 

 ##########
 File path: model/pipeline/src/main/proto/beam_runner_api.proto
 ##########
 @@ -442,35 +442,30 @@ message ParDoPayload {
   // be placed in the pipeline requirements.
   map<string, StateSpec> state_specs = 4;
 
-  // (Optional) A mapping of local timer names to timer specifications.
-  // If this is set, the stateful processing requirement should also
-  // be placed in the pipeline requirements.
-  map<string, TimerSpec> timer_specs = 5;
-
-  // (Optional) A mapping of local timer family names to timer specifications.
-  // If this is set, the stateful processing requirement should also
-  // be placed in the pipeline requirements.
-  map<string, TimerFamilySpec> timer_family_specs = 9;
+  // (Optional) A mapping of local timer family names to timer family
+  // specifications. If this is set, the stateful processing requirement should
+  // also be placed in the pipeline requirements.
+  map<string, TimerFamilySpec> timer_family_specs = 8;
 
 Review comment:
   I would suggest not changing the proto tag numbers otherwise you'll have a more difficult import/update issue with Google.
   
   It will be much easier as a separate change to renumber them in increasing order, so timer_family_specs is 5, restriction_coder_id is 6, requests_finalization is 7, ... (making the import into Google much simpler to do).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] lukecwik commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604604470
 
 
   Run Java Flink PortableValidatesRunner Streaming

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload
URL: https://github.com/apache/beam/pull/11216#discussion_r398011965
 
 

 ##########
 File path: sdks/python/apache_beam/transforms/userstate.py
 ##########
 @@ -141,6 +143,13 @@ def __init__(self, name, time_domain):
   def __repr__(self):
     return '%s(%s)' % (self.__class__.__name__, self.name)
 
+  def to_timer_family_runner_api(self, context):
 
 Review comment:
   I'm working on the Java changes meanwhile having some python tests to run to catch any breakages. Changing to `to_runner_api` is the next step. We should also remove `TimerSpec` definition from proto as long as nowhere else uses it. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz merged pull request #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz merged pull request #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604795592
 
 
   Run Java Flink PortableValidatesRunner Streaming

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604688942
 
 
   Run Java_Examples_Dataflow PreCommit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604598488
 
 
   Retest all please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] suztomo commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
suztomo commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-606158291
 
 
   @boyuanzz @lukecwik  beam_PostCommit_Java_ValidatesRunner_Dataflow started failing after this PR:
   https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/4719/
   
   History:
   
   <img width="690" alt="Screen Shot 2020-03-30 at 14 07 08" src="https://user-images.githubusercontent.com/28604/77946079-c3421880-728f-11ea-9b0a-e5b55b078db8.png">
   
   I see `Run Dataflow ValidatesRunner` was not tested in this PR.
   
   ```
   00:27:33 > Task :runners:google-cloud-dataflow-java:validatesRunnerLegacyWorkerTest
   00:28:48 
   00:28:48 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerWithMultipleTimerFamilyUnbounded FAILED
   00:28:48     java.lang.RuntimeException at ParDoTest.java:4770
   00:28:48         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4770
   00:42:57 
   00:42:57 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerFamilyEventTimeUnbounded FAILED
   00:42:57     java.lang.RuntimeException at ParDoTest.java:4708
   00:42:57         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4708
   ```
   
   I appreciate if you can check whether the failure is related to this PR.
   
   Created an issue: https://issues.apache.org/jira/browse/BEAM-9636

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-606161054
 
 
   > @boyuanzz @lukecwik Run Dataflow ValidatesRunner (beam_PostCommit_Java_ValidatesRunner_Dataflow) started failing after this PR:
   > https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/4719/
   > 
   > History:
   > 
   > <img alt="Screen Shot 2020-03-30 at 14 07 08" width="690" src="https://user-images.githubusercontent.com/28604/77946079-c3421880-728f-11ea-9b0a-e5b55b078db8.png">
   > 
   > ```
   > 00:27:33 > Task :runners:google-cloud-dataflow-java:validatesRunnerLegacyWorkerTest
   > 00:28:48 
   > 00:28:48 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerWithMultipleTimerFamilyUnbounded FAILED
   > 00:28:48     java.lang.RuntimeException at ParDoTest.java:4770
   > 00:28:48         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4770
   > 00:42:57 
   > 00:42:57 org.apache.beam.sdk.transforms.ParDoTest$TimerFamilyTests > testTimerFamilyEventTimeUnbounded FAILED
   > 00:42:57     java.lang.RuntimeException at ParDoTest.java:4708
   > 00:42:57         Caused by: java.lang.IllegalArgumentException at ParDoTest.java:4708
   > ```
   > 
   > I appreciate if you can check whether the failure is related to this PR.
   > 
   > Created an issue: https://issues.apache.org/jira/browse/BEAM-9636
   
   Thanks for the notification. Looking.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload

Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from ParDoPayload
URL: https://github.com/apache/beam/pull/11216#discussion_r398691696
 
 

 ##########
 File path: sdks/python/apache_beam/transforms/userstate.py
 ##########
 @@ -141,6 +143,13 @@ def __init__(self, name, time_domain):
   def __repr__(self):
     return '%s(%s)' % (self.__class__.__name__, self.name)
 
+  def to_timer_family_runner_api(self, context):
 
 Review comment:
   +1 on TimerSpec removal

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604673655
 
 
   Run Java Flink PortableValidatesRunner Streaming

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604806948
 
 
   All tests passed. Going to merge it. Thanks for your help!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#discussion_r398801152
 
 

 ##########
 File path: sdks/python/apache_beam/transforms/userstate.py
 ##########
 @@ -141,6 +143,13 @@ def __init__(self, name, time_domain):
   def __repr__(self):
     return '%s(%s)' % (self.__class__.__name__, self.name)
 
+  def to_timer_family_runner_api(self, context):
 
 Review comment:
   You can rename this method to `to_runner_api` and remove the method below.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604734389
 
 
   Run Portable_Python PreCommit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604673310
 
 
   Run Java PreCommit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604622943
 
 
   Run Java Flink PortableValidatesRunner Streaming

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto

Posted by GitBox <gi...@apache.org>.
boyuanzz commented on issue #11216: [BEAM-9562] Remove TimerSpec from Proto
URL: https://github.com/apache/beam/pull/11216#issuecomment-604673717
 
 
   Run Portable_Python PreCommit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services