You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "shunping (via GitHub)" <gi...@apache.org> on 2025/08/14 03:19:17 UTC
[PR] Fix a bug in cogbk for not using registered coder. [beam]
shunping opened a new pull request, #35862:
URL: https://github.com/apache/beam/pull/35862
We received an internal report (internal bug id: 430560535) that cogbk not honoring custom coders.
Below is the code to reproduce.
```Python
"""Test Beam's use of coders for keys in CoGroupByKey."""
import apache_beam as beam
from apache_beam.options import pipeline_options
class _Unpicklable:
def __init__(self, value):
self.value = value
def __getstate__(self):
raise NotImplementedError()
def __setstate__(self, state):
raise NotImplementedError()
def __repr__(self):
return f"Unpicklable({self.value})"
class _UnpicklableCoder(beam.coders.Coder):
"""."""
def encode(self, value):
return str(value.value).encode()
def decode(self, encoded):
return _Unpicklable(int(encoded.decode()))
def to_type_hint(self):
return _Unpicklable
def is_deterministic(self):
return True
beam.coders.registry.register_coder(_Unpicklable, _UnpicklableCoder)
def pipeline_fn(root):
values = [_Unpicklable(i) for i in range(5)]
xs = root | beam.Create(values) | beam.WithKeys(lambda x: x)
return (
{'x': xs}
| beam.CoGroupByKey()
| beam.FlatMapTuple(lambda k, tagged: (k.value, tagged['x'][0].value * 2))
| beam.LogElements()
)
def main():
options = pipeline_options.PipelineOptions(
runner='DirectRunner', direct_num_workers=1, type_check_additional='all'
)
with beam.Pipeline(options=options) as pipeline:
_ = pipeline_fn(pipeline)
if __name__ == '__main__':
main()
```
Running this code will result in a "NotImplementedError", because pickled coder rather than the registered coder is used.
My current PR can fix this problem by propagating type hints correctly.
Related PR: #33932
--
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] Fix a bug in cogbk for not using registered coder. [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping merged PR #35862:
URL: https://github.com/apache/beam/pull/35862
--
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] Fix a bug in cogbk for not using registered coder. [beam]
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on code in PR #35862:
URL: https://github.com/apache/beam/pull/35862#discussion_r2276541016
##########
sdks/python/apache_beam/transforms/util.py:
##########
@@ -266,7 +267,9 @@ def collect_values(key, tagged_values):
]
| Flatten(pipeline=self.pipeline)
| GroupByKey()
- | MapTuple(collect_values))
+ | MapTuple(collect_values).with_input_types(
Review Comment:
will this break some internal tests?
--
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] Fix a bug in cogbk for not using registered coder. [beam]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #35862:
URL: https://github.com/apache/beam/pull/35862#issuecomment-3186770009
## [Codecov](https://app.codecov.io/gh/apache/beam/pull/35862?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 56.69%. Comparing base ([`0e47f80`](https://app.codecov.io/gh/apache/beam/commit/0e47f80865e038f3fb3cc53c09e8b607c072cf52?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e731616`](https://app.codecov.io/gh/apache/beam/commit/e7316161ff11df3b4b71ff7c4af88712f8989f18?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #35862 +/- ##
=========================================
Coverage 56.69% 56.69%
Complexity 3380 3380
=========================================
Files 1204 1205 +1
Lines 184118 184164 +46
Branches 3507 3507
=========================================
+ Hits 104383 104413 +30
- Misses 76406 76422 +16
Partials 3329 3329
```
| [Flag](https://app.codecov.io/gh/apache/beam/pull/35862/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [python](https://app.codecov.io/gh/apache/beam/pull/35862/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.96% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/beam/pull/35862?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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] Fix a bug in cogbk for not using registered coder. [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35862:
URL: https://github.com/apache/beam/pull/35862#issuecomment-3186912082
Assigning reviewers:
R: @liferoad 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] Fix a bug in cogbk for not using registered coder. [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #35862:
URL: https://github.com/apache/beam/pull/35862#issuecomment-3188791447
cc'ed @jrmccluskey
--
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] Fix a bug in cogbk for not using registered coder. [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #35862:
URL: https://github.com/apache/beam/pull/35862#discussion_r2276656565
##########
sdks/python/apache_beam/transforms/util.py:
##########
@@ -266,7 +267,9 @@ def collect_values(key, tagged_values):
]
| Flatten(pipeline=self.pipeline)
| GroupByKey()
- | MapTuple(collect_values))
+ | MapTuple(collect_values).with_input_types(
Review Comment:
Similar to #33932, it is possible to break some tests because we are changing the coder for this particular step, from a universal pickled coder to the actual coder for the element.
However, the impact radius will be much smaller than the previous PR, as cogbk is used less often than Reshuffle.
--
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] Fix a bug in cogbk for not using registered coder. [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #35862:
URL: https://github.com/apache/beam/pull/35862#issuecomment-3188865943
Internal tests are green. Merging.
--
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