[BEAM-9562] Remove TimerSpec from Proto by boyuanzz · Pull Request #11216 · apache/beam

boyuanzz · 2020-03-25T03:21:49Z

This PR removes TimerSpec from Proto. Timer will be translated into TimerFamilySpec over fnapi.

R: @lukecwik

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) 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.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	SDK	Apex	Dataflow	Gearpump	Samza
Go		---	---	---	---
Java
Python		---		---	---
XLang	---	---	---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website
Non-portable
Portable	---		---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

lukecwik

Remember that you can run the Python portable validates runner tests using Flink (./gradlew :runners:flink:1.9:job-server:validatesPortableRunner). This should give you good coverage as to whether the change as a whole works.

lukecwik · 2020-03-25T15:21:18Z

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).

lukecwik · 2020-03-25T15:24:27Z

shouldn't this method replace to_runner_api?

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.

+1 on TimerSpec removal

You can rename this method to to_runner_api and remove the method below.

lukecwik · 2020-03-25T15:25:51Z

Suggested change
# TODO(BEAM-9602): Provide support of dynamic timer
# TODO(BEAM-9602): Provide support for dynamic timer

boyuanzz · 2020-03-26T18:23:46Z

Retest all please

lukecwik · 2020-03-26T18:31:39Z

You can rename this method to to_runner_api and remove the method below.

lukecwik · 2020-03-26T18:33:16Z

Run Java Flink PortableValidatesRunner Streaming

boyuanzz · 2020-03-26T18:57:44Z

Run Java Flink PortableValidatesRunner Streaming

boyuanzz · 2020-03-26T20:39:59Z

Run Java PreCommit

boyuanzz · 2020-03-26T20:40:14Z

Run JavaPortabilityApi PreCommit

boyuanzz · 2020-03-26T20:40:40Z

Run Java Flink PortableValidatesRunner Streaming

boyuanzz · 2020-03-26T20:40:48Z

Run Portable_Python PreCommit

boyuanzz · 2020-03-26T20:40:57Z

Run Python2_PVR_Flink PreCommit

boyuanzz · 2020-03-26T21:11:22Z

Run Python PreCommit

boyuanzz · 2020-03-26T21:11:33Z

Run Java_Examples_Dataflow PreCommit

boyuanzz · 2020-03-26T23:15:26Z

Run Portable_Python PreCommit

boyuanzz · 2020-03-27T03:27:29Z

Run Java Flink PortableValidatesRunner Streaming

boyuanzz · 2020-03-27T04:20:52Z

All tests passed. Going to merge it. Thanks for your help!

suztomo · 2020-03-30T18:13:54Z

@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:

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

boyuanzz · 2020-03-30T18:19:27Z

@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:
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.

probot-autolabelerBot added model python labels Mar 25, 2020

omarsmak mentioned this pull request Mar 25, 2020
CAMEL-14780 camel-undertow: Enhance SPI API for HttpHandler change apache/camel#3677
Merged

lukecwik reviewed Mar 25, 2020
View reviewed changes

probot-autolabelerBot added core dataflow java runners fn-execution labels Mar 25, 2020

boyuanzz force-pushed the timer_spec branch from 907b569 to 3cd45e0Compare March 26, 2020 18:23

boyuanzz changed the title ~~[BEAM-9562] Remove TimerSpec from ParDoPayload~~[BEAM-9562] Remove TimerSpec from ProtoMar 26, 2020

lukecwik requested changes Mar 26, 2020
View reviewed changes

boyuanzz force-pushed the timer_spec branch from 3cd45e0 to 60d298aCompare March 26, 2020 18:57

lukecwik approved these changes Mar 26, 2020
View reviewed changes

Remove TimeSpec from proto
39703c6

boyuanzz force-pushed the timer_spec branch from 60d298a to 39703c6Compare March 27, 2020 03:26

boyuanzz merged commit 33d78c5 into apache:masterMar 27, 2020

suztomo mentioned this pull request Mar 30, 2020
[BEAM-9444] Use GCP Libraries BOM for Google Cloud Dependencies #11156
Closed
4 tasks

	# TODO(BEAM-9602): Provide support of dynamic timer
	# TODO(BEAM-9602): Provide support for dynamic timer

Uh oh!

Conversation

boyuanzz commented Mar 25, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

R: @lukecwik

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

Uh oh!

lukecwik left a comment

Choose a reason for hiding this comment

Uh oh!

lukecwikMar 25, 2020

Choose a reason for hiding this comment

Uh oh!

lukecwikMar 25, 2020

Choose a reason for hiding this comment

Uh oh!

boyuanzzMar 25, 2020

Choose a reason for hiding this comment

Uh oh!

lukecwikMar 26, 2020

Choose a reason for hiding this comment

Uh oh!

lukecwikMar 26, 2020

Choose a reason for hiding this comment

Uh oh!

lukecwikMar 25, 2020

Choose a reason for hiding this comment

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

lukecwikMar 26, 2020

Choose a reason for hiding this comment

Uh oh!

lukecwik commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 26, 2020

Uh oh!

boyuanzz commented Mar 27, 2020

Uh oh!

boyuanzz commented Mar 27, 2020

Uh oh!

suztomo commented Mar 30, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boyuanzz commented Mar 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

boyuanzz commented Mar 25, 2020•
edited
Loading

suztomo commented Mar 30, 2020•
edited
Loading