Skip to content

Commit c54cc2b

Browse files
authored
Add flag for disabling dill check in coders. (#36453)
* init. * Trigger test. * Remove pipeline option. * BQ file loads test. * Fixes. * Fix test.
1 parent 08b4800 commit c54cc2b

7 files changed

Lines changed: 50 additions & 41 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"comment": "Modify this file in a trivial way to cause this test suite to run",
3+
"revision": 1
4+
}

.github/workflows/beam_PreCommit_Python_Dill.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,9 @@ jobs:
106106
arguments: |
107107
-Pposargs="${{
108108
contains(matrix.os, 'self-hosted') &&
109-
'apache_beam/internal/ apache_beam/ml/ apache_beam/transforms/ apache_beam/typehints/ apache_beam/runners/portability/ -m (uses_dill and not require_docker_in_docker)' ||
110-
'apache_beam/internal/ apache_beam/ml/ apache_beam/transforms/ apache_beam/typehints/ apache_beam/runners/portability/ -m (uses_dill and require_docker_in_docker)'
111-
}}" \
109+
'apache_beam/internal/ apache_beam/io/gcp/ apache_beam/options/ apache_beam/transforms/ apache_beam/typehints/ apache_beam/runners/portability/ -m (uses_dill and not require_docker_in_docker)' ||
110+
'apache_beam/internal/ apache_beam/io/gcp/ apache_beam/options/ apache_beam/transforms/ apache_beam/typehints/ apache_beam/runners/portability/ -m (uses_dill and require_docker_in_docker)'
111+
}}" \
112112
-PpythonVersion=${{ matrix.python_version }}
113113
- name: Archive Python Test Results
114114
uses: actions/upload-artifact@v4

sdks/python/apache_beam/coders/coders.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,8 +1004,16 @@ def to_type_hint(self):
10041004
return Any
10051005

10061006

1007-
def _should_force_use_dill(update_compat_version):
1007+
def _should_force_use_dill(registry):
1008+
# force_dill_deterministic_coders is for testing purposes. If there is a
1009+
# DeterministicFastPrimitivesCoder in the pipeline graph but the dill
1010+
# encoding path is not really triggered dill does not have to be installed.
1011+
# and this check can be skipped.
1012+
if getattr(registry, 'force_dill_deterministic_coders', False):
1013+
return True
1014+
10081015
from apache_beam.transforms.util import is_v1_prior_to_v2
1016+
update_compat_version = registry.update_compatibility_version
10091017
if not update_compat_version:
10101018
return False
10111019

@@ -1035,11 +1043,11 @@ def _update_compatible_deterministic_fast_primitives_coder(coder, step_label):
10351043
relative filepaths in code objects and dynamic functions.
10361044
"""
10371045
from apache_beam.coders import typecoders
1038-
update_compat_version = typecoders.registry.update_compatibility_version
1039-
if _should_force_use_dill(update_compat_version):
1046+
1047+
if _should_force_use_dill(typecoders.registry):
10401048
return DeterministicFastPrimitivesCoder(coder, step_label)
10411049
return DeterministicFastPrimitivesCoderV2(
1042-
coder, step_label, update_compat_version)
1050+
coder, step_label, typecoders.registry.update_compatibility_version)
10431051

10441052

10451053
class FastPrimitivesCoder(FastCoder):

sdks/python/apache_beam/io/gcp/bigquery_file_loads_test.py

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,6 @@
6363
except ImportError:
6464
raise unittest.SkipTest('GCP dependencies are not installed')
6565

66-
try:
67-
import dill
68-
except ImportError:
69-
dill = None
70-
7166
_LOGGER = logging.getLogger(__name__)
7267

7368
_DESTINATION_ELEMENT_PAIRS = [
@@ -411,13 +406,6 @@ def test_partition_files_dofn_size_split(self):
411406
label='CheckSinglePartition')
412407

413408

414-
def maybe_skip(compat_version):
415-
if compat_version and not dill:
416-
raise unittest.SkipTest(
417-
'Dill dependency not installed which is required for compat_version'
418-
' <= 2.67.0')
419-
420-
421409
class TestBigQueryFileLoads(_TestCaseWithTempDirCleanUp):
422410
def test_trigger_load_jobs_with_empty_files(self):
423411
destination = "project:dataset.table"
@@ -497,9 +485,9 @@ def test_records_traverse_transform_with_mocks(self):
497485
param(compat_version=None),
498486
param(compat_version="2.64.0"),
499487
])
500-
@pytest.mark.uses_dill
501488
def test_reshuffle_before_load(self, compat_version):
502-
maybe_skip(compat_version)
489+
from apache_beam.coders import typecoders
490+
typecoders.registry.force_dill_deterministic_coders = True
503491
destination = 'project1:dataset1.table1'
504492

505493
job_reference = bigquery_api.JobReference()
@@ -525,13 +513,17 @@ def test_reshuffle_before_load(self, compat_version):
525513
validate=False,
526514
temp_file_format=bigquery_tools.FileFormat.JSON)
527515

528-
options = PipelineOptions(update_compatibility_version=compat_version)
516+
options = PipelineOptions(
517+
update_compatibility_version=compat_version,
518+
# Disable unrelated compatibility change.
519+
force_cloudpickle_deterministic_coders=True)
529520
# Need to test this with the DirectRunner to avoid serializing mocks
530521
with TestPipeline('DirectRunner', options=options) as p:
531522
_ = p | beam.Create(_ELEMENTS) | transform
532523

533524
reshuffle_before_load = compat_version is None
534525
assert transform.reshuffle_before_load == reshuffle_before_load
526+
typecoders.registry.force_dill_deterministic_coders = False
535527

536528
def test_load_job_id_used(self):
537529
job_reference = bigquery_api.JobReference()
@@ -1008,7 +1000,9 @@ def dynamic_destination_resolver(element, *side_inputs):
10081000
])
10091001
def test_triggering_frequency(
10101002
self, is_streaming, with_auto_sharding, compat_version):
1011-
maybe_skip(compat_version)
1003+
from apache_beam.coders import typecoders
1004+
typecoders.registry.force_dill_deterministic_coders = True
1005+
10121006
destination = 'project1:dataset1.table1'
10131007

10141008
job_reference = bigquery_api.JobReference()
@@ -1114,6 +1108,8 @@ def __call__(self):
11141108
label='CheckDestinations')
11151109
assert_that(jobs, equal_to(expected_jobs), label='CheckJobs')
11161110

1111+
typecoders.registry.force_dill_deterministic_coders = False
1112+
11171113

11181114
class BigQueryFileLoadsIT(unittest.TestCase):
11191115

sdks/python/apache_beam/options/pipeline_options.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,18 @@ def _add_argparse_args(cls, parser):
874874
'their condition met. Some operations, such as GroupByKey, disallow '
875875
'this. This exists for cases where such loss is acceptable and for '
876876
'backwards compatibility. See BEAM-9487.')
877+
parser.add_argument(
878+
'--force_cloudpickle_deterministic_coders',
879+
default=False,
880+
action='store_true',
881+
help=(
882+
'Force the use of cloudpickle-based deterministic coders '
883+
'instead of dill-based coders, even when '
884+
'update_compatibility_version would normally trigger dill usage '
885+
'for backward compatibility. This flag overrides automatic coder '
886+
'selection to always use the modern cloudpickle serialization '
887+
' path. Warning: May break pipeline update compatibility with '
888+
' SDK versions prior to 2.68.0.'))
877889

878890
def validate(self, unused_validator):
879891
errors = []

sdks/python/apache_beam/transforms/util_test.py

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@
9292
from apache_beam.utils.windowed_value import PaneInfoTiming
9393
from apache_beam.utils.windowed_value import WindowedValue
9494

95-
try:
96-
import dill
97-
except ImportError:
98-
dill = None
99-
10095
try:
10196
from google.cloud import secretmanager
10297
except ImportError:
@@ -131,13 +126,6 @@ def is_deterministic(self):
131126
return True
132127

133128

134-
def maybe_skip(compat_version):
135-
if compat_version and not dill:
136-
raise unittest.SkipTest(
137-
'Dill dependency not installed which is required for compat_version'
138-
' <= 2.67.0')
139-
140-
141129
class CoGroupByKeyTest(unittest.TestCase):
142130
def test_co_group_by_key_on_tuple(self):
143131
with TestPipeline() as pipeline:
@@ -1219,10 +1207,10 @@ def test_reshuffle_streaming_global_window_with_buckets(self):
12191207
param(compat_version=None),
12201208
param(compat_version="2.64.0"),
12211209
])
1222-
@pytest.mark.uses_dill
12231210
def test_reshuffle_custom_window_preserves_metadata(self, compat_version):
12241211
"""Tests that Reshuffle preserves pane info."""
1225-
maybe_skip(compat_version)
1212+
from apache_beam.coders import typecoders
1213+
typecoders.registry.force_dill_deterministic_coders = True
12261214
element_count = 12
12271215
timestamp_value = timestamp.Timestamp(0)
12281216
l = [
@@ -1286,7 +1274,6 @@ def test_reshuffle_custom_window_preserves_metadata(self, compat_version):
12861274
expected_timestamp, [GlobalWindow()],
12871275
PANE_INFO_UNKNOWN)
12881276
])
1289-
12901277
options = PipelineOptions(update_compatibility_version=compat_version)
12911278
options.view_as(StandardOptions).streaming = True
12921279

@@ -1317,16 +1304,17 @@ def test_reshuffle_custom_window_preserves_metadata(self, compat_version):
13171304
equal_to(expected),
13181305
label='CheckMetadataPreserved',
13191306
reify_windows=True)
1307+
typecoders.registry.force_dill_deterministic_coders = False
13201308

13211309
@parameterized.expand([
13221310
param(compat_version=None),
13231311
param(compat_version="2.64.0"),
13241312
])
1325-
@pytest.mark.uses_dill
13261313
def test_reshuffle_default_window_preserves_metadata(self, compat_version):
13271314
"""Tests that Reshuffle preserves timestamp, window, and pane info
13281315
metadata."""
1329-
maybe_skip(compat_version)
1316+
from apache_beam.coders import typecoders
1317+
typecoders.registry.force_dill_deterministic_coders = True
13301318
no_firing = PaneInfo(
13311319
is_first=True,
13321320
is_last=True,
@@ -1400,6 +1388,7 @@ def test_reshuffle_default_window_preserves_metadata(self, compat_version):
14001388
equal_to(expected),
14011389
label='CheckMetadataPreserved',
14021390
reify_windows=True)
1391+
typecoders.registry.force_dill_deterministic_coders = False
14031392

14041393
@pytest.mark.it_validatesrunner
14051394
def test_reshuffle_preserves_timestamps(self):

sdks/python/tox.ini

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,4 +590,4 @@ commands =
590590
# Log dill version for debugging
591591
/bin/sh -c "pip freeze | grep -E dill"
592592
# Run all dill-specific tests
593-
/bin/sh -c 'pytest -o junit_suite_name={envname} --junitxml=pytest_{envname}.xml -n 1 -m uses_dill {posargs}; ret=$?; [ $ret = 5 ] && exit 0 || exit $ret'
593+
/bin/sh -c 'pytest -o junit_suite_name={envname} --junitxml=pytest_{envname}.xml -n 1 -m uses_dill -vv -ra {posargs}; ret=$?; [ $ret = 5 ] && exit 0 || exit $ret'

0 commit comments

Comments
 (0)