lostluck opened a new pull request, #33840:
URL: https://github.com/apache/beam/pull/33840
While implementing Triggers + Panes for Prism, I ran into inexplicable
failing tests.
It turns out that portable encoding of panes in the protos *does not* match
the implementations in the Python, and Java SDKs, or the internal Dataflow
implementation, and never has in the 5 years since it was added to the protocol
buffers.
Specifically, the bits for first and last panes are swapped with first being
the 7 indexed bit, and last being the 6 indexed bit.
Compare Python and Java
https://github.com/apache/beam/blob/master/sdks/python/apache_beam/utils/windowed_value.py#L92
https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/windowing/PaneInfo.java#L134
To Go's current implementation.
https://github.com/apache/beam/blob/release-2.62.0/sdks/go/pkg/beam/core/graph/coder/panes.go#L32
It is worth noting that the Go implemenation didn't have the swap some years
ago when it was first added in
https://github.com/apache/beam/commit/b0e9f2638cbaca822ee6a58bd8ec0e61db8e799e
but it wasn't unit tested at the time.
But due to a different error in the implementation, the bit swap occurred in
https://github.com/apache/beam/commit/657caa88c011cae818377f64f02afe9d18614da5
where it was made to match the spec in the protocol buffers on top of fixing
the other error.
This error wasn't caught until now due to Triggers (and thus Panes) being
among the last features implemented in the SDK, and the least tested due to no
real portable implementations of TestStream necessary to exercise them. The Go
SDK doesn't currently provide facilities to assert against specific pane
properties, and it is unlikely for there to be user code for this since it
seems like the Go SDK also doesn't propagate panes properly. See
https://github.com/apache/beam/pull/31174/files and #31153 .
------------------------
Thank you for your contribution! Follow this checklist to help us
incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example:
`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, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] 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://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit
[https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more
information about GitHub Actions CI or the [workflows
README](https://github.com/apache/beam/blob/master/.github/workflows/README.md)
to see a list of phrases to trigger workflows.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
