Skip to content
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

Fix custom coders not being used in Reshuffle (non global window) #33363

Merged

Conversation

shunping
Copy link
Contributor

The follow-up PR of #33339 to fix the non-global window scenario.

fixes #33356

@shunping shunping marked this pull request as draft December 12, 2024 04:17
@shunping shunping changed the title Reshuffle output hint non global window [WIP] Reshuffle output hint non global window Dec 12, 2024
@shunping shunping changed the title [WIP] Reshuffle output hint non global window [WIP] Fix custom coders not being used in Reshuffle (non global window) Dec 12, 2024
@shunping shunping changed the title [WIP] Fix custom coders not being used in Reshuffle (non global window) Fix custom coders not being used in Reshuffle (non global window) Dec 14, 2024
@shunping shunping marked this pull request as ready for review December 14, 2024 01:05
@shunping
Copy link
Contributor Author

R: @jrmccluskey @robertwb

@shunping
Copy link
Contributor Author

Run Flink Container PreCommit

Copy link
Contributor

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

Copy link
Contributor

@robertwb robertwb left a comment

Choose a reason for hiding this comment

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

Thanks!

@classmethod
def from_type_hint(cls, typehint, registry):
# type: (Any, CoderRegistry) -> WindowedValueCoder
return cls(registry.get_coder(typehint.inner_type))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a TODO to pass the right coder for the Window as well (so we don't fall back to PickleCoder to encode the windows?) This would require making TypedWindowedValue have two type parameters, the second being optional, and might get messy, so can be deferred for now so we can get this in to unbreak things.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I'll just drop the note now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you Robert!

@robertwb robertwb merged commit 2aab9cd into apache:master Dec 14, 2024
68 checks passed
shunping added a commit to shunping/beam that referenced this pull request Dec 18, 2024
- Fix custom coder not being used in Reshuffle (global window) (apache#33339)
- Fix custom coders not being used in Reshuffle (non global window) apache#33363
- Add missing to_type_hint to WindowedValueCoder apache#33403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Reshuffle failed to use custom coders in non-global window scenario
2 participants