You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "alexmalao1 (via GitHub)" <gi...@apache.org> on 2025/11/04 17:46:36 UTC
[PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
alexmalao1 opened a new pull request, #36722:
URL: https://github.com/apache/beam/pull/36722
Added source window to `default_window_mapping_fn` for side inputs. Existing window functions use timestamp to assign windows for side inputs, but custom windows not based on time would need to propagate the source window.
Addresses #36719
------------------------
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-4120211365
From the linter:
```
Running flake8...
0
Running isort...
ERROR: /runner/_work/beam/beam/sdks/python/test-suites/tox/pycommon/build/srcs/sdks/python/apache_beam/transforms/sideinputs_test.py Imports are incorrectly sorted and/or formatted.
Command exited with non-zero status 1
--- /runner/_work/beam/beam/sdks/python/test-suites/tox/pycommon/build/srcs/sdks/python/apache_beam/transforms/sideinputs_test.py:before 2026-03-24 17:20:31.290684
805.85user 19.38system 1:53.65elapsed 726%CPU (0avgtext+0avgdata 835104maxresident)k
2320inputs+2944outputs (2major+2084773minor)pagefaults 0swaps
+++ /runner/_work/beam/beam/sdks/python/test-suites/tox/pycommon/build/srcs/sdks/python/apache_beam/transforms/sideinputs_test.py:after 2026-03-24 17:27:54.505575
@@ -39,9 +39,9 @@
from apache_beam.testing.util import equal_to
from apache_beam.testing.util import equal_to_per_window
from apache_beam.transforms import Map
+from apache_beam.transforms import sideinputs
from apache_beam.transforms import trigger
from apache_beam.transforms import window
-from apache_beam.transforms import sideinputs
from apache_beam.utils.timestamp import Timestamp
```
Could you please try running `./gradlew lint` to find issues like this before submitting?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "alexmalao1 (via GitHub)" <gi...@apache.org>.
alexmalao1 commented on code in PR #36722:
URL: https://github.com/apache/beam/pull/36722#discussion_r2794934497
##########
sdks/python/apache_beam/transforms/sideinputs_test.py:
##########
@@ -489,6 +491,32 @@ def process(
assert_that(results, equal_to([(num_records, expected_fingerprint)]))
pipeline.run()
+ def test_default_window_mapping_fn_source_window(self):
Review Comment:
Added the required dummy `_getTimestampFromProto` and `get_window_coder` methods for the string id window examples.
Test should pass now.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3744171786
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3938696599
Reminder, please take a look at this pr: @damccorm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-4067330907
Reminder, please take a look at this pr: @shunping @damccorm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3817314499
Reminder, please take a look at this pr: @damccorm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "alexmalao1 (via GitHub)" <gi...@apache.org>.
alexmalao1 commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3773421202
Reopening review, added relevant unit test.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3487322936
## Summary of Changes
Hello @alexmalao1, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request improves the flexibility of windowing for side inputs in Apache Beam by providing the full source window object to the `AssignContext`. This allows for more sophisticated and custom window assignment strategies that might rely on properties of the source window beyond just its maximum timestamp, addressing a limitation for non-timestamp-based windowing scenarios.
### Highlights
* **Enhanced Window Assignment for Side Inputs**: The `AssignContext` within `default_window_mapping_fn` now explicitly exposes the `source_window` object, in addition to its `max_timestamp`.
* **Support for Custom Windowing**: This change enables custom window functions for side inputs that are not solely dependent on timestamps, allowing for more flexible window propagation logic.
<details>
<summary><b>Using Gemini Code Assist</b></summary>
<br>
The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.
<b>Invoking Gemini</b>
You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page.
Feature | Command | Description
--- | --- | ---
Code Review | `/gemini review` | Performs a code review for the current pull request in its current state.
Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help | `/gemini help` | Displays a list of available commands.
<b>Customization</b>
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).
<b>Limitations & Feedback</b>
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).
<b>You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).</b>
</details>
[^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #36722:
URL: https://github.com/apache/beam/pull/36722#discussion_r2849181269
##########
sdks/python/apache_beam/transforms/sideinputs_test.py:
##########
@@ -489,6 +491,39 @@ def process(
assert_that(results, equal_to([(num_records, expected_fingerprint)]))
pipeline.run()
+ def test_default_window_mapping_fn_source_window(self):
+ """Test that the default window mapping function will propagate the
+ source window when attempting to assign context.
+ """
+ class StringIDWindow(window.BoundedWindow):
+ """A window defined by an arbitrary string ID."""
+ def __init__(self, window_id: str):
+ super().__init__(self._getTimestampFromProto())
+ self.id = window_id
+
+ @staticmethod
+ def _getTimestampFromProto() -> Timestamp:
+ return Timestamp(micros=0)
+
+ class StringIDWindows(window.NonMergingWindowFn):
+ """ A windowing function that assigns each element a window with ID."""
+ def assign(
+ self, assign_context: window.WindowFn.AssignContext
+ ) -> Iterable[StringIDWindow]:
Review Comment:
Looks like linting checks are still failing - https://github.com/apache/beam/actions/runs/22365640103/job/64730757498?pr=36722 - could you please take a look?
You should be able to reproduce locally by running `./gradlew :pythonLintPreCommit`
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3871436139
Reminder, please take a look at this pr: @damccorm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3527545119
Reminder, please take a look at this pr: @damccorm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "alisha-1000 (via GitHub)" <gi...@apache.org>.
alisha-1000 commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3833800400
Thanks for the detailed analysis — this is very helpful.
The hypothesis around AutoValueSchema field ordering during state restoration makes sense, especially given that it only reproduces on job updates (not drain). That aligns with schema-based state decoding paths being exercised only during update.
It might be worth confirming whether the temporary `KafkaSourceDescriptor.create()` workaround is still needed, or if removing it (as suggested in #36357) resolves the ordering mismatch in newer Beam versions.
Happy to help test or validate a fix once there’s guidance from maintainers.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #36722:
URL: https://github.com/apache/beam/pull/36722#discussion_r2742459568
##########
sdks/python/apache_beam/transforms/sideinputs_test.py:
##########
@@ -489,6 +491,32 @@ def process(
assert_that(results, equal_to([(num_records, expected_fingerprint)]))
pipeline.run()
+ def test_default_window_mapping_fn_source_window(self):
Review Comment:
Looks like this is failing in CI with:
```
<testcase classname="apache_beam.transforms.sideinputs_test.SideInputsTest" name="test_default_window_mapping_fn_source_window" time="0.001">
<failure message="TypeError: Can't instantiate abstract class StringIDWindows with abstract method get_window_coder">self = <apache_beam.transforms.sideinputs_test.SideInputsTest testMethod=test_default_window_mapping_fn_source_window> def test_default_window_mapping_fn_source_window(self): """Test that the default window mapping function will propagate the source window when attempting to assign context. """ class StringIDWindow(window.BoundedWindow): """A window defined by an arbitrary string ID.""" def __init__(self, window_id: str): super().__init__(self._getTimestampFromProto()) self.id = window_id class StringIDWindows(window.NonMergingWindowFn): """ A windowing function that assigns each element a window with ID.""" def assign( self, assign_context: window.WindowFn.AssignContext ) -> Iterable[StringIDWindow]: if assign_context.element is None: return [assign_context.window] return [StringIDWindow(str(assign_context.element))] > mapping_fn = sideinputs.default_window_mapping
_fn(StringIDWindows()) E TypeError: Can't instantiate abstract class StringIDWindows with abstract method get_window_coder apache_beam/transforms/sideinputs_test.py:513: TypeError</failure>
</testcase>
```
Could you please take a look?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "hjtran (via GitHub)" <gi...@apache.org>.
hjtran commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3492560507
Nice find @alexmalao1. Could you include a test? Probably easiest to first put up a separate PR for adding the non-time-based custom window we've implemented and then creating a test using that
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3997227204
Reminder, please take a look at this pr: @damccorm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3945631811
waiting on author
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3772724527
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3493165820
> Nice find @alexmalao1. Could you include a test? Probably easiest to first put up a separate PR for adding the non-time-based custom window we've implemented and then creating a test using that
+1 to adding a test, otherwise this LGTM though - thanks for the find
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #36722:
URL: https://github.com/apache/beam/pull/36722#discussion_r2962233901
##########
1cmy.pdb:
##########
@@ -0,0 +1,5288 @@
+HEADER OXYGEN TRANSPORT 18-SEP-92 1CMY
Review Comment:
Looks like this was not meant to be added
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-4068028381
Hey @alexmalao1 it looks like linting checks are still failing - could you please try to fix these?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3487834951
Assigning reviewers:
R: @damccorm for label python.
Note: If you would like to opt out of this review, comment `assign to next reviewer`.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #36722: Exposed source window to AssignContext in window_mapping_fn for side inputs
URL: https://github.com/apache/beam/pull/36722
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3872182561
waiting on author
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-3527939939
waiting on author
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #36722:
URL: https://github.com/apache/beam/pull/36722#discussion_r2805660289
##########
sdks/python/apache_beam/transforms/sideinputs_test.py:
##########
@@ -489,6 +491,39 @@ def process(
assert_that(results, equal_to([(num_records, expected_fingerprint)]))
pipeline.run()
+ def test_default_window_mapping_fn_source_window(self):
+ """Test that the default window mapping function will propagate the
+ source window when attempting to assign context.
+ """
+ class StringIDWindow(window.BoundedWindow):
+ """A window defined by an arbitrary string ID."""
+ def __init__(self, window_id: str):
+ super().__init__(self._getTimestampFromProto())
+ self.id = window_id
+
+ @staticmethod
+ def _getTimestampFromProto() -> Timestamp:
+ return Timestamp(micros=0)
+
+ class StringIDWindows(window.NonMergingWindowFn):
+ """ A windowing function that assigns each element a window with ID."""
+ def assign(
+ self, assign_context: window.WindowFn.AssignContext
+ ) -> Iterable[StringIDWindow]:
Review Comment:
Would you mind adjusting this type hint? It looks like this is causing linting to fail: https://github.com/apache/beam/actions/runs/21918758108/job/63292969740?pr=36722
```
apache_beam/transforms/sideinputs_test.py:514: error: List item 0 has incompatible type "BoundedWindow | None"; expected "StringIDWindow" [list-item]
```
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-4068029036
waiting on author
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "alexmalao1 (via GitHub)" <gi...@apache.org>.
alexmalao1 commented on code in PR #36722:
URL: https://github.com/apache/beam/pull/36722#discussion_r2849013225
##########
sdks/python/apache_beam/transforms/sideinputs_test.py:
##########
@@ -489,6 +491,39 @@ def process(
assert_that(results, equal_to([(num_records, expected_fingerprint)]))
pipeline.run()
+ def test_default_window_mapping_fn_source_window(self):
+ """Test that the default window mapping function will propagate the
+ source window when attempting to assign context.
+ """
+ class StringIDWindow(window.BoundedWindow):
+ """A window defined by an arbitrary string ID."""
+ def __init__(self, window_id: str):
+ super().__init__(self._getTimestampFromProto())
+ self.id = window_id
+
+ @staticmethod
+ def _getTimestampFromProto() -> Timestamp:
+ return Timestamp(micros=0)
+
+ class StringIDWindows(window.NonMergingWindowFn):
+ """ A windowing function that assigns each element a window with ID."""
+ def assign(
+ self, assign_context: window.WindowFn.AssignContext
+ ) -> Iterable[StringIDWindow]:
Review Comment:
Updated to `window.BoundedWindow`
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36722:
URL: https://github.com/apache/beam/pull/36722#issuecomment-4023394992
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @shunping for label python.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Exposed source window to AssignContext in window_mapping_fn for side inputs [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm merged PR #36722:
URL: https://github.com/apache/beam/pull/36722
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org