fix(sdk.v2): Fix regression on optional inputs (#6905)
* Fix regression on optional inputs * add release note
This commit is contained in:
Chen Sun
GitHub
parent
8f809c56b1
commit
9ee4534aef
|
|
@ -23,6 +23,7 @@
|
|||
* Fix importer ignoring reimport setting, and switch to Protobuf.Value for import uri [\#6827](https://github.com/kubeflow/pipelines/pull/6827)
|
||||
* Fix display name support for groups [\#6832](https://github.com/kubeflow/pipelines/pull/6832)
|
||||
* Remove redundant check in set_gpu_limit [\#6866](https://github.com/kubeflow/pipelines/pull/6866)
|
||||
* Fix regression on optional inputs [\#6905](https://github.com/kubeflow/pipelines/pull/6905)
|
||||
|
||||
## Documentation Updates
|
||||
|
||||
|
|
|
|||
|
|
@ -143,11 +143,11 @@ class CompilerCliTests(unittest.TestCase):
|
|||
def test_pipeline_with_loops(self):
|
||||
self._test_compile_py_to_json('pipeline_with_loops')
|
||||
|
||||
# TODO: re-enable the test, fix optional input support
|
||||
# TODO: re-enable the test, fix regression on loop
|
||||
# def test_pipeline_with_nested_loops(self):
|
||||
# self._test_compile_py_to_json('pipeline_with_nested_loops')
|
||||
|
||||
# TODO: re-enable the test, fix optional input support
|
||||
# TODO: re-enable the test, fix regression on loop
|
||||
# def test_pipeline_with_loops_and_conditions(self):
|
||||
# self._test_compile_py_to_json('pipeline_with_loops_and_conditions')
|
||||
|
||||
|
|
@ -176,9 +176,8 @@ class CompilerCliTests(unittest.TestCase):
|
|||
# def test_pipeline_with_env(self):
|
||||
# self._test_compile_py_to_json('pipeline_with_env')
|
||||
|
||||
# TODO: re-enable the test, fix optional input support
|
||||
# def test_v2_component_with_optional_inputs(self):
|
||||
# self._test_compile_py_to_json('v2_component_with_optional_inputs')
|
||||
def test_v2_component_with_optional_inputs(self):
|
||||
self._test_compile_py_to_json('v2_component_with_optional_inputs')
|
||||
|
||||
def test_pipeline_with_gcpc_types(self):
|
||||
self._test_compile_py_to_json('pipeline_with_gcpc_types')
|
||||
|
|
|
|||
|
|
@ -75,9 +75,6 @@
|
|||
},
|
||||
"message": {
|
||||
"parameterType": "STRING"
|
||||
},
|
||||
"num_steps": {
|
||||
"parameterType": "NUMBER_INTEGER"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
@ -223,11 +220,6 @@
|
|||
"outputParameterKey": "output_parameter_path",
|
||||
"producerTask": "preprocess"
|
||||
}
|
||||
},
|
||||
"num_steps": {
|
||||
"runtimeValue": {
|
||||
"constant": 100.0
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1,82 +1,77 @@
|
|||
{
|
||||
"pipelineSpec": {
|
||||
"components": {
|
||||
"comp-component-op": {
|
||||
"executorLabel": "exec-component-op",
|
||||
"inputDefinitions": {
|
||||
"parameters": {
|
||||
"input1": {
|
||||
"parameterType": "STRING"
|
||||
},
|
||||
"input2": {
|
||||
"parameterType": "STRING"
|
||||
}
|
||||
"components": {
|
||||
"comp-component-op": {
|
||||
"executorLabel": "exec-component-op",
|
||||
"inputDefinitions": {
|
||||
"parameters": {
|
||||
"input1": {
|
||||
"parameterType": "STRING"
|
||||
},
|
||||
"input2": {
|
||||
"parameterType": "STRING"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"deploymentSpec": {
|
||||
"executors": {
|
||||
"exec-component-op": {
|
||||
"container": {
|
||||
"args": [
|
||||
"--executor_input",
|
||||
"{{$}}",
|
||||
"--function_to_execute",
|
||||
"component_op"
|
||||
],
|
||||
"command": [
|
||||
"sh",
|
||||
"-c",
|
||||
"\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip || python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1 python3 -m pip install --quiet --no-warn-script-location 'kfp==1.8.6' && \"$0\" \"$@\"\n",
|
||||
"sh",
|
||||
"-ec",
|
||||
"program_path=$(mktemp -d)\nprintf \"%s\" \"$0\" > \"$program_path/ephemeral_component.py\"\npython3 -m kfp.v2.components.executor_main --component_module_path \"$program_path/ephemeral_component.py\" \"$@\"\n",
|
||||
"\nimport kfp\nfrom kfp.v2 import dsl\nfrom kfp.v2.dsl import *\nfrom typing import *\n\ndef component_op(\n input1: str = 'default value',\n input2: Optional[str] = None,\n input3: Optional[str] = None,\n):\n print(f'input1: {input1}, type: {type(input1)}')\n print(f'input2: {input2}, type: {type(input2)}')\n print(f'input3: {input3}, type: {type(input3)}')\n\n"
|
||||
],
|
||||
"image": "python:3.7"
|
||||
}
|
||||
}
|
||||
},
|
||||
"deploymentSpec": {
|
||||
"executors": {
|
||||
"exec-component-op": {
|
||||
"container": {
|
||||
"args": [
|
||||
"--executor_input",
|
||||
"{{$}}",
|
||||
"--function_to_execute",
|
||||
"component_op"
|
||||
],
|
||||
"command": [
|
||||
"sh",
|
||||
"-c",
|
||||
"\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip || python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1 python3 -m pip install --quiet --no-warn-script-location 'kfp==1.8.9' && \"$0\" \"$@\"\n",
|
||||
"sh",
|
||||
"-ec",
|
||||
"program_path=$(mktemp -d)\nprintf \"%s\" \"$0\" > \"$program_path/ephemeral_component.py\"\npython3 -m kfp.v2.components.executor_main --component_module_path \"$program_path/ephemeral_component.py\" \"$@\"\n",
|
||||
"\nimport kfp\nfrom kfp.v2 import dsl\nfrom kfp.v2.dsl import *\nfrom typing import *\n\ndef component_op(\n input1: str = 'default value',\n input2: Optional[str] = None,\n input3: Optional[str] = None,\n):\n print(f'input1: {input1}, type: {type(input1)}')\n print(f'input2: {input2}, type: {type(input2)}')\n print(f'input3: {input3}, type: {type(input3)}')\n\n"
|
||||
],
|
||||
"image": "python:3.7"
|
||||
}
|
||||
}
|
||||
},
|
||||
"pipelineInfo": {
|
||||
"name": "v2-component-optional-input"
|
||||
},
|
||||
"root": {
|
||||
"dag": {
|
||||
"tasks": {
|
||||
"component-op": {
|
||||
"cachingOptions": {
|
||||
"enableCache": true
|
||||
},
|
||||
"componentRef": {
|
||||
"name": "comp-component-op"
|
||||
},
|
||||
"inputs": {
|
||||
"parameters": {
|
||||
"input1": {
|
||||
"runtimeValue": {
|
||||
"constant": "Hello"
|
||||
}
|
||||
},
|
||||
"input2": {
|
||||
"runtimeValue": {
|
||||
"constant": "World"
|
||||
}
|
||||
}
|
||||
},
|
||||
"pipelineInfo": {
|
||||
"name": "v2-component-optional-input"
|
||||
},
|
||||
"root": {
|
||||
"dag": {
|
||||
"tasks": {
|
||||
"component-op": {
|
||||
"cachingOptions": {
|
||||
"enableCache": true
|
||||
},
|
||||
"componentRef": {
|
||||
"name": "comp-component-op"
|
||||
},
|
||||
"inputs": {
|
||||
"parameters": {
|
||||
"input1": {
|
||||
"runtimeValue": {
|
||||
"constant": "Hello"
|
||||
}
|
||||
},
|
||||
"input2": {
|
||||
"runtimeValue": {
|
||||
"constant": "World"
|
||||
}
|
||||
}
|
||||
},
|
||||
"taskInfo": {
|
||||
"name": "component-op"
|
||||
}
|
||||
},
|
||||
"taskInfo": {
|
||||
"name": "component-op"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"schemaVersion": "2.1.0",
|
||||
"sdkVersion": "kfp-1.8.6"
|
||||
}
|
||||
},
|
||||
"runtimeConfig": {
|
||||
"gcsOutputDirectory": "dummy_root"
|
||||
}
|
||||
"schemaVersion": "2.1.0",
|
||||
"sdkVersion": "kfp-1.8.9"
|
||||
}
|
||||
|
|
@ -62,23 +62,18 @@ class BaseComponent(metaclass=abc.ABCMeta):
|
|||
f'{self.name}() got multiple values for argument "{k}".')
|
||||
task_inputs[k] = v
|
||||
|
||||
# Fill in default value if there was no user provided value
|
||||
for input_name, input_spec in (self.component_spec.inputs or
|
||||
{}).items():
|
||||
if input_spec.default is not None and input_name not in task_inputs:
|
||||
task_inputs[input_name] = input_spec.default
|
||||
|
||||
missing_arguments = [
|
||||
input_name for input_name in (self.component_spec.inputs or {})
|
||||
if input_name not in task_inputs
|
||||
input_name for input_name, input_spec in (
|
||||
self.component_spec.inputs or {}).items()
|
||||
if input_name not in task_inputs and not input_spec.optional
|
||||
]
|
||||
if missing_arguments:
|
||||
argument_or_arguments = 'argument' if len(
|
||||
missing_arguments) == 1 else 'arguments'
|
||||
arguments = ','.join(missing_arguments)
|
||||
arguments = ', '.join(missing_arguments)
|
||||
|
||||
raise TypeError(
|
||||
f'{self.name}() missing {len(missing_arguments)} required positional '
|
||||
f'{self.name}() missing {len(missing_arguments)} required '
|
||||
f'{argument_or_arguments}: {arguments}.')
|
||||
|
||||
return pipeline_task.create_pipeline_task(
|
||||
|
|
|
|||
|
|
@ -43,9 +43,14 @@ component_op = TestComponent(
|
|||
],
|
||||
),
|
||||
inputs={
|
||||
'input1': structures.InputSpec(type='String'),
|
||||
'input2': structures.InputSpec(type='Integer'),
|
||||
'input3': structures.InputSpec(type='Float', default=3.14),
|
||||
'input1':
|
||||
structures.InputSpec(type='String'),
|
||||
'input2':
|
||||
structures.InputSpec(type='Integer'),
|
||||
'input3':
|
||||
structures.InputSpec(type='Float', default=3.14),
|
||||
'input4':
|
||||
structures.InputSpec(type='Optional[Float]', default=None),
|
||||
},
|
||||
outputs={
|
||||
'output1': structures.OutputSpec(name='output1', type='String'),
|
||||
|
|
@ -59,7 +64,7 @@ class BaseComponentTest(unittest.TestCase):
|
|||
def test_instantiate_component_with_keyword_arguments(
|
||||
self, mock_create_pipeline_task):
|
||||
|
||||
component_op(input1='hello', input2=100, input3=1.23)
|
||||
component_op(input1='hello', input2=100, input3=1.23, input4=3.21)
|
||||
|
||||
mock_create_pipeline_task.assert_called_once_with(
|
||||
component_spec=component_op.component_spec,
|
||||
|
|
@ -67,6 +72,7 @@ class BaseComponentTest(unittest.TestCase):
|
|||
'input1': 'hello',
|
||||
'input2': 100,
|
||||
'input3': 1.23,
|
||||
'input4': 3.21,
|
||||
})
|
||||
|
||||
@patch.object(pipeline_task, 'create_pipeline_task', autospec=True)
|
||||
|
|
@ -80,33 +86,31 @@ class BaseComponentTest(unittest.TestCase):
|
|||
arguments={
|
||||
'input1': 'hello',
|
||||
'input2': 100,
|
||||
'input3': '3.14',
|
||||
})
|
||||
|
||||
def test_instantiate_component_with_positional_arugment(self):
|
||||
with self.assertRaisesRegex(
|
||||
TypeError,
|
||||
'Components must be instantiated using keyword arguments. Positional '
|
||||
'parameters are not allowed \(found 3 such parameters for component '
|
||||
'"component_1"\).'):
|
||||
'Components must be instantiated using keyword arguments.'
|
||||
' Positional parameters are not allowed \(found 3 such'
|
||||
' parameters for component "component_1"\).'):
|
||||
component_op('abc', 1, 2.3)
|
||||
|
||||
def test_instantiate_component_with_unexpected_keyword_arugment(self):
|
||||
with self.assertRaisesRegex(
|
||||
TypeError,
|
||||
'component_1\(\) got an unexpected keyword argument "input4".'):
|
||||
component_op(input1='abc', input2=1, input3=2.3, input4='extra')
|
||||
'component_1\(\) got an unexpected keyword argument "input0".'):
|
||||
component_op(input1='abc', input2=1, input3=2.3, input0='extra')
|
||||
|
||||
def test_instantiate_component_with_missing_arugments(self):
|
||||
with self.assertRaisesRegex(
|
||||
TypeError,
|
||||
'component_1\(\) missing 1 required positional argument: input1.'
|
||||
):
|
||||
'component_1\(\) missing 1 required argument: input1.'):
|
||||
component_op(input2=1)
|
||||
|
||||
with self.assertRaisesRegex(
|
||||
TypeError,
|
||||
'component_1\(\) missing 2 required positional arguments: input1,input2.'
|
||||
'component_1\(\) missing 2 required arguments: input1, input2.'
|
||||
):
|
||||
component_op()
|
||||
|
||||
|
|
|
|||
|
|
@ -226,19 +226,18 @@ def extract_component_interface(func: Callable) -> structures.ComponentSpec:
|
|||
else:
|
||||
io_name = _maybe_make_unique(io_name, input_names)
|
||||
input_names.add(io_name)
|
||||
input_spec = structures.InputSpec(
|
||||
type=type_struct, description=doc_dict.get(parameter.name))
|
||||
if parameter.default is not inspect.Parameter.empty:
|
||||
# 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
|
||||
try:
|
||||
input_spec.default = parameter.default
|
||||
except Exception as ex:
|
||||
warnings.warn(
|
||||
'Could not serialize the default value of the'
|
||||
' parameter "{}". {}'.format(parameter.name, ex))
|
||||
input_spec = structures.InputSpec(
|
||||
type=type_struct,
|
||||
description=doc_dict.get(parameter.name),
|
||||
default=parameter.default,
|
||||
)
|
||||
else:
|
||||
input_spec = structures.InputSpec(
|
||||
type=type_struct,
|
||||
description=doc_dict.get(parameter.name),
|
||||
)
|
||||
|
||||
inputs[io_name] = input_spec
|
||||
|
||||
#Analyzing the return type annotations.
|
||||
|
|
|
|||
|
|
@ -38,11 +38,23 @@ class InputSpec(BaseModel):
|
|||
type: The type of the input.
|
||||
default: Optional; the default value for the input.
|
||||
description: Optional: the user description of the input.
|
||||
optional: Wether the input is optional. An input is optional when it has
|
||||
an explicit default value.
|
||||
"""
|
||||
# TODO(ji-yaqi): Add logic to cast default value into the specified type.
|
||||
type: str
|
||||
default: Optional[Union[str, int, float, bool, dict, list]] = None
|
||||
description: Optional[str] = None
|
||||
_optional: bool = pydantic.PrivateAttr()
|
||||
|
||||
def __init__(self, **data):
|
||||
super().__init__(**data)
|
||||
# An input is optional if a default value is explicitly specified.
|
||||
self._optional = 'default' in data
|
||||
|
||||
@property
|
||||
def optional(self) -> bool:
|
||||
return self._optional
|
||||
|
||||
|
||||
class OutputSpec(BaseModel):
|
||||
|
|
@ -578,7 +590,7 @@ class ComponentSpec(BaseModel):
|
|||
"""
|
||||
with open(output_file, 'a') as output_file:
|
||||
json_component = self.json(
|
||||
exclude_none=True, exclude_unset=True, by_alias=True)
|
||||
exclude_none=False, exclude_unset=True, by_alias=True)
|
||||
yaml_file = yaml.safe_dump(
|
||||
json.loads(json_component),
|
||||
default_flow_style=None,
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ V1_YAML_IF_PLACEHOLDER = textwrap.dedent("""\
|
|||
V2_YAML_IF_PLACEHOLDER = textwrap.dedent("""\
|
||||
name: component_if
|
||||
inputs:
|
||||
optional_input_1: {type: String}
|
||||
optional_input_1: {type: String, default: null}
|
||||
implementation:
|
||||
container:
|
||||
image: alpine
|
||||
|
|
@ -75,7 +75,9 @@ V2_COMPONENT_SPEC_IF_PLACEHOLDER = structures.ComponentSpec(
|
|||
'default',
|
||||
]))
|
||||
])),
|
||||
inputs={'optional_input_1': structures.InputSpec(type='String')},
|
||||
inputs={
|
||||
'optional_input_1': structures.InputSpec(type='String', default=None)
|
||||
},
|
||||
)
|
||||
|
||||
V1_YAML_CONCAT_PLACEHOLDER = textwrap.dedent("""\
|
||||
|
|
|
|||
Loading…
Reference in New Issue