Skip to content

Conversation

@kennknowles
Copy link
Member

This is a preliminary change to reduce the number of files to review in #34902

For the user-facing API I think we would like getPaneInfo to be appropriately named. Notably, a PaneInfo record is not the same as a "pane". A "pane" is a name for a single triggered output of a window (probably we tried to be too clever with names) but the PaneInfo does not uniquely identify the pane. In Python SDK this is the public field WindowedVAlue.pane_info so it also makes the SDKs match better. In Go SDK it is pane but it is not blocking the current change.


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.

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)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

@kennknowles
Copy link
Member Author

R: @stankiewicz

since you noticed this was a lot of the files in that PR

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@kennknowles
Copy link
Member Author

Also @talatuyarer commented the same :-)

Copy link
Contributor

@stankiewicz stankiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for splitting the PR :)

@kennknowles
Copy link
Member Author

It looks like there might be a need to do this in multiple steps or with a complex dance, due to containers and/or reflection. I think the adapter method that I put in #35140 will work, if I merge that one first.

@kennknowles
Copy link
Member Author

Thanks for reviewing, anyhow! I will do this in a small step after #35140. If you can review that it would be great. It is "large" but has almost no actual content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants