You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "claudevdm (via GitHub)" <gi...@apache.org> on 2025/09/08 23:43:32 UTC
[PR] Make dill optional [beam]
claudevdm opened a new pull request, #36093:
URL: https://github.com/apache/beam/pull/36093
**Please** add a meaningful description for your change here
------------------------
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] Make dill optional [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3275904028
/gemini review
--
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] Make dill optional [beam]
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3286694862
some flaky ones from the logs:
```
# py310
[gw1] FAILED apache_beam/io/gcp/pubsub_integration_test.py::PubSubIntegrationTest::test_streaming_with_attributes
```
```
# py39
[gw0] FAILED apache_beam/io/gcp/pubsub_integration_test.py::PubSubIntegrationTest::test_batch_write_data_only
```
--
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] Make dill optional [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3276066451
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`
--
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] Make dill optional [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3268554348
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of 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] Make dill optional [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3276600423
Failing tests look like flakes and unrelated.
--
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] Make dill optional [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm merged PR #36093:
URL: https://github.com/apache/beam/pull/36093
--
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] Make dill optional [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3276062678
R: @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] Make dill optional [beam]
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3286857848
> PubSubIntegrationTest
trying https://github.com/apache/beam/pull/36140
--
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] Make dill optional [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #36093:
URL: https://github.com/apache/beam/pull/36093#discussion_r2337469851
##########
sdks/python/apache_beam/internal/pickler.py:
##########
@@ -74,6 +78,12 @@ def load_session(file_path):
def set_library(selected_library=DEFAULT_PICKLE_LIB):
""" Sets pickle library that will be used. """
global desired_pickle_lib
+
+ if selected_library == USE_DILL and not dill_pickler:
+ if not dill_pickler:
+ raise ImportError(
+ "Dill is not installed. To use dill pickler, install apache-beam with"
+ " the dill extras package e.g. apache-beam[dill].")
Review Comment:
The nested `if not dill_pickler:` check is redundant because the outer `if` condition already verifies that `dill_pickler` is falsy. You can remove the inner `if` block to simplify the code.
```python
if selected_library == USE_DILL and not dill_pickler:
raise ImportError(
"Dill is not installed. To use dill pickler, install apache-beam with"
" the dill extras package e.g. apache-beam[dill].")
```
##########
sdks/python/apache_beam/coders/coders.py:
##########
@@ -900,6 +898,12 @@ def to_type_hint(self):
class DillCoder(_PickleCoderBase):
"""Coder using dill's pickle functionality."""
+ def __init__(self):
+ if not dill:
+ raise RuntimeError("This pipeline contains a DillCoder which requires " \
+ "the dill package. Install the dill package with the dill extra " \
+ "e.g. apache-beam[dill]")
Review Comment:
For multi-line strings, it's more idiomatic in Python to use implicit string concatenation within parentheses rather than using backslashes for line continuation. This improves readability and is less prone to formatting errors.
```suggestion
raise RuntimeError(
"This pipeline contains a DillCoder which requires the dill package. "
"Install the dill package with the dill extra e.g. apache-beam[dill]")
```
##########
sdks/python/apache_beam/options/pipeline_options_validator.py:
##########
@@ -196,6 +205,25 @@ def validate_gcs_path(self, view, arg_name):
return self._validate_error(self.ERR_INVALID_GCS_OBJECT, arg, arg_name)
return []
+ def validate_pickle_library(self, view):
+ """Validates a GCS path against gs://bucket/object URI format."""
Review Comment:
The docstring for this function appears to be a copy-paste error and is misleading. It should describe that the function validates the `pickle_library` option, not a GCS path.
```suggestion
"""Validates the pickle_library option."""
```
--
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] Make dill optional [beam]
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #36093:
URL: https://github.com/apache/beam/pull/36093#discussion_r2350167317
##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -1619,7 +1619,7 @@ def _add_argparse_args(cls, parser):
help=(
'Chooses which pickle library to use. Options are dill, '
'cloudpickle or default.'),
- choices=['cloudpickle', 'default', 'dill'])
+ choices=['cloudpickle', 'default', 'dill', 'dill_unsafe'])
Review Comment:
possible other ways to name this concept:
dill_any
dill_untested
dill_any_version
--
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] Make dill optional [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3286607543
> Failing postcommit tests look related to #36027
@liferoad 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] Make dill optional [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3286587021
Failing postcommit tests look related to https://github.com/apache/beam/pull/36027
--
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] Make dill optional [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3286685359
I do agree that this PR isn't causing the failures though, so 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
Re: [PR] Make dill optional [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on PR #36093:
URL: https://github.com/apache/beam/pull/36093#issuecomment-3411965647
https://github.com/apache/beam/issues/21298
--
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