diff --git a/sdk/python/kfp/components/_data_passing.py b/sdk/python/kfp/components/_data_passing.py index 12615e52a77..80fef0d08d7 100644 --- a/sdk/python/kfp/components/_data_passing.py +++ b/sdk/python/kfp/components/_data_passing.py @@ -182,10 +182,6 @@ def serialize_value(value, type_name: str) -> str: str(e), )) - serialized_value = str(value) - warnings.warn('There are no registered serializers from type "{}" to type "{}", so the value will be serializers as string "{}".'.format( - str(type(value).__name__), + raise TypeError('There are no registered serializers for type "{}".'.format( str(type_name), - serialized_value), - ) - return serialized_value + )) diff --git a/sdk/python/kfp/components/_python_op.py b/sdk/python/kfp/components/_python_op.py index 9a89dbff7ae..42ba21d3cfa 100644 --- a/sdk/python/kfp/components/_python_op.py +++ b/sdk/python/kfp/components/_python_op.py @@ -35,6 +35,7 @@ from pathlib import Path import typing from typing import Callable, Generic, List, TypeVar, Union +import warnings T = TypeVar('T') @@ -332,7 +333,10 @@ def annotation_to_type_struct(annotation): input_spec.optional = True if parameter.default is not None: outer_type_name = list(type_struct.keys())[0] if isinstance(type_struct, dict) else type_struct - input_spec.default = serialize_value(parameter.default, outer_type_name) + try: + input_spec.default = serialize_value(parameter.default, outer_type_name) + except Exception as ex: + warnings.warn('Could not serialize the default value of the parameter "{}". {}'.format(parameter.name, ex)) input_spec._passing_style = passing_style input_spec._parameter_name = parameter.name inputs.append(input_spec) @@ -356,7 +360,6 @@ def annotation_to_type_struct(annotation): outputs.append(output_spec) # Deprecated dict-based way of declaring multiple outputs. Was only used by the @component decorator elif isinstance(return_ann, dict): - import warnings warnings.warn( "The ability to specify multiple outputs using the dict syntax has been deprecated." "It will be removed soon after release 0.1.32." diff --git a/sdk/python/kfp/components_tests/test_components.py b/sdk/python/kfp/components_tests/test_components.py index 91e60e24d29..0845f0016e5 100644 --- a/sdk/python/kfp/components_tests/test_components.py +++ b/sdk/python/kfp/components_tests/test_components.py @@ -693,6 +693,33 @@ def test_check_task_object_no_output_attribute_when_multiple_outputs(self): self.assertFalse(hasattr(task, 'output')) + def test_prevent_passing_unserializable_objects_as_argument(self): + component_text = textwrap.dedent('''\ + inputs: + - {name: input 1} + - {name: input 2} + implementation: + container: + image: busybox + command: + - prog + - {inputValue: input 1} + - {inputPath: input 2} + ''' + ) + component = comp.load_component_from_text(component_text) + # Passing normal values to component + task1 = component(input_1="value 1", input_2="value 2") + # Passing unserializable values to component + with self.assertRaises(TypeError): + component(input_1=task1, input_2="value 2") + with self.assertRaises(TypeError): + component(input_1=open, input_2="value 2") + with self.assertRaises(TypeError): + component(input_1="value 1", input_2=task1) + with self.assertRaises(TypeError): + component(input_1="value 1", input_2=open) + def test_check_type_validation_of_task_spec_outputs(self): producer_component_text = '''\ outputs: diff --git a/sdk/python/kfp/dsl/_component_bridge.py b/sdk/python/kfp/dsl/_component_bridge.py index c87dd576481..13a5585c6d3 100644 --- a/sdk/python/kfp/dsl/_component_bridge.py +++ b/sdk/python/kfp/dsl/_component_bridge.py @@ -34,6 +34,8 @@ def _create_container_op_from_component_and_arguments( dsl.types.verify_type_compatibility(reference_type, input_type, 'Incompatible argument passed to the input "{}" of component "{}": '.format(input_name, component_spec.name)) arguments[input_name] = str(argument_value) + if isinstance(argument_value, dsl.ContainerOp): + raise TypeError('ContainerOp object was passed to component as an input argument. Pass a single output instead.') resolved_cmd = _resolve_command_line_and_paths( component_spec=component_spec, diff --git a/sdk/python/tests/dsl/component_bridge_tests.py b/sdk/python/tests/dsl/component_bridge_tests.py index c1c8ab45567..19c4a90f723 100644 --- a/sdk/python/tests/dsl/component_bridge_tests.py +++ b/sdk/python/tests/dsl/component_bridge_tests.py @@ -243,3 +243,26 @@ def test_reusable_component_warnings(self): with self.assertWarnsRegex(DeprecationWarning, expected_regex='reusable'): kfp.dsl.ContainerOp(name='name', image='image') + + def test_prevent_passing_container_op_as_argument(self): + component_text = textwrap.dedent('''\ + inputs: + - {name: input 1} + - {name: input 2} + implementation: + container: + image: busybox + command: + - prog + - {inputValue: input 1} + - {inputPath: input 2} + ''' + ) + component = load_component_from_text(component_text) + # Passing normal values to component + task1 = component(input_1="value 1", input_2="value 2") + # Passing unserializable values to component + with self.assertRaises(TypeError): + component(input_1=task1, input_2="value 2") + with self.assertRaises(TypeError): + component(input_1="value 1", input_2=task1)