Skip to content

Conversation

@kennknowles
Copy link
Member

This has no user-visible effect unless they have to declare a variable of type Trigger, etc, since we use factory methods where extra type parameters are just a warning. For this PR, I have deliberately not cleared out such parameters from examples or integration tests, to be sure that nothing is broken. I will follow up with that.

This is in service of the runner API in that triggers move towards a cross-language cross-runner syntax and clearing out this type parameter makes it easier to proceed with further refactors.

@kennknowles
Copy link
Member Author

R: @bjchambers

@bjchambers
Copy link
Contributor

Looks fine. There may be more narrowing of scope we can do but it can wait.

It would be good to be able to write a trigger which depended on the window and was checked at application time. Unfortunately, since we don't carry W around on the PCollection, we can't statically check this unless the trigger is specified at the same time the windowing is. We can tighten the API or do some construction-time checks if needed, if/when it comes up.

LGTM

@kennknowles
Copy link
Member Author

Thanks for the review!

@asfgit asfgit merged commit de9f10c into apache:master Apr 12, 2016
asfgit pushed a commit that referenced this pull request Apr 12, 2016
@kennknowles kennknowles deleted the Trigger-window branch April 19, 2016 17:23
swegner pushed a commit to swegner/beam that referenced this pull request Apr 22, 2016
iemejia pushed a commit to iemejia/beam that referenced this pull request Jan 12, 2018
mareksimunek pushed a commit to mareksimunek/beam that referenced this pull request May 9, 2018
 [euphoria-hadoop] remove sneakythrows
damccorm added a commit that referenced this pull request Dec 9, 2022
…lf-hosted runners (#23135)

* Updating build_playground_frontend Workflow(#168)

Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>

* Added master changes in build_playground_frontend to avoid merge conflicts

* Switching trigger to pull_request (#260)

* Switching to pull_request

* Removing ref from checkout

Co-authored-by: Elias Segundo Antonio <eliassegundo.segundo@gmail.com>
Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>
Co-authored-by: elink22 <103056145+elink22@users.noreply.github.com>
Co-authored-by: Danny McCormick <dannymccormick@google.com>
lostluck pushed a commit to lostluck/beam that referenced this pull request Dec 22, 2022
…lf-hosted runners (apache#23135)

* Updating build_playground_frontend Workflow(apache#168)

Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>

* Added master changes in build_playground_frontend to avoid merge conflicts

* Switching trigger to pull_request (apache#260)

* Switching to pull_request

* Removing ref from checkout

Co-authored-by: Elias Segundo Antonio <eliassegundo.segundo@gmail.com>
Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>
Co-authored-by: elink22 <103056145+elink22@users.noreply.github.com>
Co-authored-by: Danny McCormick <dannymccormick@google.com>
pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
Co-authored-by: Tres Seaver <tseaver@palladion.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants