Skip to content

Commit

Permalink
feat(sdk): Components - Prevent passing unserializable objects to com…
Browse files Browse the repository at this point in the history
…ponents. Fixes #4040 (#4496)
  • Loading branch information
Ark-kun authored Sep 16, 2020
1 parent 41ce68e commit 0332584
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 8 deletions.
8 changes: 2 additions & 6 deletions sdk/python/kfp/components/_data_passing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
))
7 changes: 5 additions & 2 deletions sdk/python/kfp/components/_python_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from pathlib import Path
import typing
from typing import Callable, Generic, List, TypeVar, Union
import warnings

T = TypeVar('T')

Expand Down Expand Up @@ -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)
Expand All @@ -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."
Expand Down
27 changes: 27 additions & 0 deletions sdk/python/kfp/components_tests/test_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions sdk/python/kfp/dsl/_component_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 23 additions & 0 deletions sdk/python/tests/dsl/component_bridge_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

0 comments on commit 0332584

Please sign in to comment.