claudevdm commented on code in PR #36090:
URL: https://github.com/apache/beam/pull/36090#discussion_r2333950449
##########
sdks/python/apache_beam/transforms/core.py:
##########
@@ -1664,7 +1664,7 @@ def with_exception_handling(
subject to change.
"""
args, kwargs = self.raw_side_inputs
- return self.label >> _ExceptionHandlingWrapper(
+ wrapper = self.label >> _ExceptionHandlingWrapper(
Review Comment:
What do you think of
1. Add resource_hints to _ExceptionHandlingWrapper.init() e.g.
```
class _ExceptionHandlingWrapper(ptransform.PTransform):
"""Implementation of ParDo.with_exception_handling."""
def __init__(
self,
...
resource_hints):
...
self._resource_hints = resource_hints
```
2. Pass resource hints in with_exception_handling
```
return self.label >> _ExceptionHandlingWrapper(
self.fn,
args,
kwargs,
main_tag,
dead_letter_tag,
exc_class,
partial,
use_subprocess,
threshold,
threshold_windowing,
timeout,
error_handler,
on_failure_callback,
allow_unsafe_userstate_in_process,
self.get_resource_hints())
```
3. _ExceptionHandlingWrapper stays similar
```
pardo = ParDo(
_ExceptionHandlingWrapperDoFn(
wrapped_fn,
self._dead_letter_tag,
self._exc_class,
self._partial,
self._on_failure_callback,
self._allow_unsafe_userstate_in_process,
),
*self._args,
**self._kwargs,
)
# This is the fix: propagate hints.
pardo.get_resource_hints().update(self._resource_hints)
...
```
This is more targeted and easier to write tests for?
##########
sdks/python/apache_beam/transforms/core.py:
##########
@@ -1664,7 +1664,7 @@ def with_exception_handling(
subject to change.
"""
args, kwargs = self.raw_side_inputs
- return self.label >> _ExceptionHandlingWrapper(
+ wrapper = self.label >> _ExceptionHandlingWrapper(
Review Comment:
Can you please add a test in
https://github.com/apache/beam/blob/dffa53f76245c4955cb0ca5e81c11df35f5eee0f/sdks/python/apache_beam/transforms/core_test.py#L334
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
