From 32e341900e6095f663612ee964842390adcfcd3a Mon Sep 17 00:00:00 2001 From: Alexey Volkov Date: Thu, 18 Apr 2019 14:35:56 -0700 Subject: [PATCH 1/2] SDK - Made ComponentSpec.implementation field optional Improved the error message when trying to convert tasks to ContainerOp. --- sdk/python/kfp/components/_dsl_bridge.py | 4 ++-- sdk/python/kfp/components/_structures.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/components/_dsl_bridge.py b/sdk/python/kfp/components/_dsl_bridge.py index 7032722cdf4..fa9c8410071 100644 --- a/sdk/python/kfp/components/_dsl_bridge.py +++ b/sdk/python/kfp/components/_dsl_bridge.py @@ -22,8 +22,8 @@ def create_container_op_from_task(task_spec: TaskSpec): argument_values = task_spec.arguments component_spec = task_spec.component_ref._component_spec - if hasattr(component_spec.implementation, 'graph'): - raise TypeError('Cannot convert graph component to ContainerOp') + if not hasattr(component_spec.implementation, 'container'): + raise TypeError('Only container component tasks can be converted to ContainerOp') inputs_dict = {input_spec.name: input_spec for input_spec in component_spec.inputs or []} container_spec = component_spec.implementation.container diff --git a/sdk/python/kfp/components/_structures.py b/sdk/python/kfp/components/_structures.py index 42c3c475512..054aecf54c1 100644 --- a/sdk/python/kfp/components/_structures.py +++ b/sdk/python/kfp/components/_structures.py @@ -237,13 +237,13 @@ class ComponentSpec(ModelBase): '''Component specification. Describes the metadata (name, description, source), the interface (inputs and outputs) and the implementation of the component.''' def __init__( self, - implementation: ImplementationType, name: Optional[str] = None, #? Move to metadata? description: Optional[str] = None, #? Move to metadata? source: Optional[SourceSpec] = None, #? Move to metadata? metadata: Optional[MetadataSpec] = None, inputs: Optional[List[InputSpec]] = None, outputs: Optional[List[OutputSpec]] = None, + implementation: Optional[ImplementationType] = None, version: Optional[str] = 'google.com/cloud/pipelines/component/v1', #tags: Optional[Set[str]] = None, ): From f993b0559b11d2de857fdfcf1923796b6c51fa41 Mon Sep 17 00:00:00 2001 From: Alexey Volkov Date: Tue, 23 Apr 2019 12:53:55 -0700 Subject: [PATCH 2/2] Switched from attribute checking to type checking --- sdk/python/kfp/components/_dsl_bridge.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/kfp/components/_dsl_bridge.py b/sdk/python/kfp/components/_dsl_bridge.py index fa9c8410071..0816eeb12f8 100644 --- a/sdk/python/kfp/components/_dsl_bridge.py +++ b/sdk/python/kfp/components/_dsl_bridge.py @@ -14,7 +14,7 @@ from collections import OrderedDict from typing import Mapping -from ._structures import ConcatPlaceholder, IfPlaceholder, InputValuePlaceholder, InputPathPlaceholder, IsPresentPlaceholder, OutputPathPlaceholder, TaskSpec +from ._structures import ContainerImplementation, ConcatPlaceholder, IfPlaceholder, InputValuePlaceholder, InputPathPlaceholder, IsPresentPlaceholder, OutputPathPlaceholder, TaskSpec from ._components import _generate_output_file_name, _default_component_name from kfp.dsl._metadata import ComponentMeta, ParameterMeta, TypeMeta, _annotation_to_typemeta @@ -22,7 +22,7 @@ def create_container_op_from_task(task_spec: TaskSpec): argument_values = task_spec.arguments component_spec = task_spec.component_ref._component_spec - if not hasattr(component_spec.implementation, 'container'): + if not isinstance(component_spec.implementation, ContainerImplementation): raise TypeError('Only container component tasks can be converted to ContainerOp') inputs_dict = {input_spec.name: input_spec for input_spec in component_spec.inputs or []}