-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Properly handle timestamp prefixing of unkown window types. #30587
Conversation
R: @Abacn |
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
This was exposed by apache#28972 when the set of "known" coders was inadvertently reduced.
4d71d6d
to
8d8f294
Compare
Gentle ping on the review request. @Abacn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested test_windowing_before_sql
locally and passed
For curiosity, what caused the window being "unknown" for that specific test?
And after this change, it seems the window is still "unknown" but the timestamp value is accessible, or I understood it incorrectly?
@@ -538,6 +570,7 @@ def maybe_length_prefixed_and_safe_coder(self, coder_id): | |||
spec=coder.spec, component_coder_ids=safe_component_ids)) | |||
return new_coder_id, safe_coder_id | |||
else: | |||
# A completely unkown coder. Wrap the entire thing in a length prefix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, the bytes encountered in XVR_Direct test was due to here
Due to the environment nesting it didn't realize that "interval window" type was understood by all SDKs, so fell back to the generic/unknown wrapping.
Yep, that's exactly it. |
It seems this PR breaks Samza and Spark VR PostCommit, https://github.com/apache/beam/actions/workflows/beam_PostCommit_Python_ValidatesRunner_Samza.yml?query=event%3Aschedule and https://github.com/apache/beam/actions/workflows/beam_PostCommit_Python_ValidatesRunner_Spark.yml?query=event%3Aschedule though Flink and Dataflow VR PostCommit remains green Error message
|
This was exposed by #28972 when the set of "known" coders was inadvertently reduced.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.