chore(sdk): hide private methods in placeholders for simpler doc (#8271)

* hide private methods in ConcatPlaceholders and IfPresentPlaceholder by renaming

* rename transform and validate methods
This commit is contained in:
Scott_Xu 2022-09-16 15:16:30 -07:00 committed by GitHub
parent d4aaa03035
commit 6443fc17dc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 94 additions and 93 deletions

View File

@ -257,7 +257,7 @@ def build_task_spec_for_task(
existing_input_name, additional_input_name))
additional_input_placeholder = placeholders.InputValuePlaceholder(
additional_input_name).to_placeholder_string()
additional_input_name)._to_placeholder_string()
input_value = input_value.replace(channel.pattern,
additional_input_placeholder)

View File

@ -166,19 +166,20 @@ class BaseModel:
"""Hook called after object is instantiated from BaseModel.
Transforms data and validates data using user-defined logic by
calling all methods prefixed with `transform_`, then all methods
prefixed with `validate_`.
calling all methods prefixed with `_transform_`, then all
methods prefixed with `_validate_`.
"""
validate_methods = [
method for method in dir(self) if
method.startswith('transform_') and callable(getattr(self, method))
method for method in dir(self)
if method.startswith('_transform_') and
callable(getattr(self, method))
]
for method in validate_methods:
getattr(self, method)()
validate_methods = [
method for method in dir(self) if method.startswith('validate_') and
callable(getattr(self, method))
method for method in dir(self) if
method.startswith('_validate_') and callable(getattr(self, method))
]
for method in validate_methods:
getattr(self, method)()

View File

@ -261,7 +261,7 @@ class TestBaseModel(unittest.TestCase):
class MyClass(base_model.BaseModel):
x: int
def validate_x(self) -> None:
def _validate_x(self) -> None:
if self.x < 2:
raise ValueError('x must be greater than 2')
@ -272,7 +272,7 @@ class TestBaseModel(unittest.TestCase):
class MyClass(base_model.BaseModel):
x: int
def validate_x(self) -> None:
def _validate_x(self) -> None:
self.x = max(self.x, 2)
mc = MyClass(x=1)
@ -282,7 +282,7 @@ class TestBaseModel(unittest.TestCase):
class MyClass(base_model.BaseModel):
x: Optional[List[int]] = None
def validate_x(self) -> None:
def _validate_x(self) -> None:
if isinstance(self.x, list) and not self.x:
self.x = None

View File

@ -365,7 +365,7 @@ def _get_command_and_args_for_containerized_component(
args = [
'--executor_input',
placeholders.ExecutorInputPlaceholder().to_placeholder_string(),
placeholders.ExecutorInputPlaceholder()._to_placeholder_string(),
'--function_to_execute',
function_name,
]
@ -480,7 +480,7 @@ def create_container_component_from_func(
container_spec)
component_spec.implementation = structures.Implementation(
container_spec_implementation)
component_spec.validate_placeholders()
component_spec._validate_placeholders()
return container_component.ContainerComponent(component_spec, func)

View File

@ -61,7 +61,7 @@ def importer(
implementation=structures.Implementation(
importer=structures.ImporterSpec(
artifact_uri=placeholders.InputValuePlaceholder(
INPUT_KEY).to_placeholder_string(),
INPUT_KEY)._to_placeholder_string(),
schema_title=type_utils.create_bundled_artifact_type(
artifact_class.schema_title, artifact_class.schema_version),
schema_version=artifact_class.schema_version,

View File

@ -252,13 +252,13 @@ class PipelineTask:
'InputValuePlaceholder.')
if input_name in args or type_utils.is_task_final_status_type(
inputs_dict[input_name].type):
return arg.to_placeholder_string()
return arg._to_placeholder_string()
input_spec = inputs_dict[input_name]
if input_spec.default is None:
raise ValueError(
f'No value provided for input: {input_name}.')
else:
return arg.to_placeholder_string()
return arg._to_placeholder_string()
elif isinstance(arg, placeholders.InputUriPlaceholder):
input_name = arg.input_name
@ -269,7 +269,7 @@ class PipelineTask:
'InputUriPlaceholder.')
if input_name in args:
return arg.to_placeholder_string()
return arg._to_placeholder_string()
input_spec = inputs_dict[input_name]
if input_spec.default is None:
raise ValueError(
@ -287,7 +287,7 @@ class PipelineTask:
'InputPathPlaceholder.')
if input_name in args:
return arg.to_placeholder_string()
return arg._to_placeholder_string()
input_spec = inputs_dict[input_name]
if input_spec._optional:
return None
@ -303,20 +303,20 @@ class PipelineTask:
f'"{outputs_dict[output_name].type}" cannot be paired with '
'OutputUriPlaceholder.')
return arg.to_placeholder_string()
return arg._to_placeholder_string()
elif isinstance(arg, (placeholders.OutputPathPlaceholder,
placeholders.OutputParameterPlaceholder)):
output_name = arg.output_name
return placeholders.OutputParameterPlaceholder(
arg.output_name).to_placeholder_string(
arg.output_name)._to_placeholder_string(
) if type_utils.is_parameter_type(
outputs_dict[output_name].type
) else placeholders.OutputPathPlaceholder(
arg.output_name).to_placeholder_string()
arg.output_name)._to_placeholder_string()
elif isinstance(arg, placeholders.Placeholder):
return arg.to_placeholder_string()
return arg._to_placeholder_string()
else:
raise TypeError(f'Unrecognized argument type: {arg}.')

View File

@ -36,7 +36,7 @@ class Placeholder(abc.ABC):
@classmethod
@abc.abstractmethod
def from_placeholder_string(cls, placeholder_string: str) -> 'Placeholder':
def _from_placeholder_string(cls, placeholder_string: str) -> 'Placeholder':
"""Converts a placeholder string to the placeholder object that
implements this method.
@ -50,7 +50,7 @@ class Placeholder(abc.ABC):
@classmethod
@abc.abstractmethod
def is_match(cls, placeholder_string: str) -> bool:
def _is_match(cls, placeholder_string: str) -> bool:
"""Checks if the placeholder string matches the placeholder object that
implements this method.
@ -63,7 +63,7 @@ class Placeholder(abc.ABC):
raise NotImplementedError
@abc.abstractmethod
def to_placeholder_string(self) -> str:
def _to_placeholder_string(self) -> str:
"""Converts the placeholder object that implements this to a
placeholder string.
@ -94,7 +94,7 @@ class RegexPlaceholderSerializationMixin(Placeholder):
_TO_PLACEHOLDER: Union[str, type(NotImplemented)] = NotImplemented
@classmethod
def is_match(cls, placeholder_string: str) -> bool:
def _is_match(cls, placeholder_string: str) -> bool:
"""Determines if the placeholder_string matches the placeholder pattern
using the _FROM_PLACEHOLDER regex.
@ -107,7 +107,7 @@ class RegexPlaceholderSerializationMixin(Placeholder):
return cls._FROM_PLACEHOLDER.match(placeholder_string) is not None
@classmethod
def from_placeholder_string(
def _from_placeholder_string(
cls,
placeholder_string: str) -> 'RegexPlaceholderSerializationMixin':
"""Converts a placeholder string into a placeholder object.
@ -134,7 +134,7 @@ class RegexPlaceholderSerializationMixin(Placeholder):
kwargs = {field_name: matches[field_name] for field_name in field_names}
return cls(**kwargs)
def to_placeholder_string(self) -> str:
def _to_placeholder_string(self) -> str:
"""Converts a placeholder object into a placeholder string.
Returns:
@ -154,7 +154,7 @@ class ExecutorInputPlaceholder(base_model.BaseModel,
_TO_PLACEHOLDER = '{{$}}'
_FROM_PLACEHOLDER = re.compile(r'\{\{\$\}\}')
def to_placeholder_string(self) -> str:
def _to_placeholder_string(self) -> str:
return self._TO_PLACEHOLDER
@ -308,7 +308,7 @@ class ConcatPlaceholder(base_model.BaseModel, Placeholder):
"""Elements to concatenate."""
@classmethod
def split_cel_concat_string(self, string: str) -> List[str]:
def _split_cel_concat_string(self, string: str) -> List[str]:
"""Splits a cel string into a list of strings, which may be normal
strings or placeholder strings.
@ -337,15 +337,15 @@ class ConcatPlaceholder(base_model.BaseModel, Placeholder):
return items
@classmethod
def is_match(cls, placeholder_string: str) -> bool:
def _is_match(cls, placeholder_string: str) -> bool:
# 'Concat' is the explicit struct for concatenation
# cel splitting handles the cases of {{input}}+{{input}} and {{input}}otherstring
return 'Concat' in json_load_nested_placeholder_aware(
placeholder_string
) or len(
ConcatPlaceholder.split_cel_concat_string(placeholder_string)) > 1
ConcatPlaceholder._split_cel_concat_string(placeholder_string)) > 1
def to_placeholder_struct(self) -> Dict[str, Any]:
def _to_placeholder_struct(self) -> Dict[str, Any]:
return {
'Concat': [
maybe_convert_placeholder_to_placeholder_string(item)
@ -353,18 +353,18 @@ class ConcatPlaceholder(base_model.BaseModel, Placeholder):
]
}
def to_placeholder_string(self) -> str:
return json.dumps(self.to_placeholder_struct())
def _to_placeholder_string(self) -> str:
return json.dumps(self._to_placeholder_struct())
@classmethod
def from_placeholder_string(cls,
placeholder_string: str) -> 'ConcatPlaceholder':
def _from_placeholder_string(
cls, placeholder_string: str) -> 'ConcatPlaceholder':
placeholder_struct = json_load_nested_placeholder_aware(
placeholder_string)
if isinstance(placeholder_struct, str):
items = [
maybe_convert_placeholder_string_to_placeholder(item)
for item in cls.split_cel_concat_string(placeholder_struct)
for item in cls._split_cel_concat_string(placeholder_struct)
]
return cls(items=items)
elif isinstance(placeholder_struct, dict):
@ -414,13 +414,13 @@ class IfPresentPlaceholder(base_model.BaseModel, Placeholder):
_aliases = {'input_name': 'inputName', 'else_': 'else'}
@classmethod
def is_match(cls, string: str) -> bool:
def _is_match(cls, string: str) -> bool:
try:
return 'IfPresent' in json.loads(string)
except json.decoder.JSONDecodeError:
return False
def to_placeholder_struct(self) -> Dict[str, Any]:
def _to_placeholder_struct(self) -> Dict[str, Any]:
then = [
maybe_convert_placeholder_to_placeholder_string(item)
for item in self.then
@ -434,11 +434,11 @@ class IfPresentPlaceholder(base_model.BaseModel, Placeholder):
struct['IfPresent']['Else'] = otherwise
return struct
def to_placeholder_string(self) -> str:
return json.dumps(self.to_placeholder_struct())
def _to_placeholder_string(self) -> str:
return json.dumps(self._to_placeholder_struct())
@classmethod
def from_placeholder_string(
def _from_placeholder_string(
cks, placeholder_string: str) -> 'IfPresentPlaceholder':
struct = json_load_nested_placeholder_aware(placeholder_string)
struct_body = struct['IfPresent']
@ -461,7 +461,7 @@ class IfPresentPlaceholder(base_model.BaseModel, Placeholder):
}
return IfPresentPlaceholder(**kwargs)
def transform_else(self) -> None:
def _transform_else(self) -> None:
"""Use None instead of empty list for optional."""
self.else_ = None if self.else_ == [] else self.else_
@ -518,8 +518,8 @@ def maybe_convert_placeholder_string_to_placeholder(
OutputParameterPlaceholder,
]
for placeholder_struct in from_string_placeholders:
if placeholder_struct.is_match(placeholder_string):
return placeholder_struct.from_placeholder_string(
if placeholder_struct._is_match(placeholder_string):
return placeholder_struct._from_placeholder_string(
placeholder_string)
return placeholder_string
@ -536,9 +536,9 @@ def maybe_convert_placeholder_to_placeholder_string(
str: The placeholder string.
"""
if isinstance(placeholder, Placeholder):
return placeholder.to_placeholder_struct() if hasattr(
return placeholder._to_placeholder_struct() if hasattr(
placeholder,
'to_placeholder_struct') else placeholder.to_placeholder_string()
'_to_placeholder_struct') else placeholder._to_placeholder_string()
return placeholder
@ -564,17 +564,17 @@ def maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
if first_key == 'inputValue':
return InputValuePlaceholder(
input_name=utils.sanitize_input_name(
first_value)).to_placeholder_string()
first_value))._to_placeholder_string()
elif first_key == 'inputPath':
return InputPathPlaceholder(
input_name=utils.sanitize_input_name(
first_value)).to_placeholder_string()
first_value))._to_placeholder_string()
elif first_key == 'inputUri':
return InputUriPlaceholder(
input_name=utils.sanitize_input_name(
first_value)).to_placeholder_string()
first_value))._to_placeholder_string()
elif first_key == 'outputPath':
outputs = component_dict['outputs']
@ -587,11 +587,11 @@ def maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
if is_parameter:
return OutputParameterPlaceholder(
output_name=utils.sanitize_input_name(
first_value)).to_placeholder_string()
first_value))._to_placeholder_string()
else:
return OutputPathPlaceholder(
output_name=utils.sanitize_input_name(
first_value)).to_placeholder_string()
first_value))._to_placeholder_string()
raise ValueError(
f'{first_value} not found in component outputs. Could not process placeholders. Component spec: {component_dict}.'
)
@ -599,7 +599,7 @@ def maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
elif first_key == 'outputUri':
return OutputUriPlaceholder(
output_name=utils.sanitize_input_name(
first_value)).to_placeholder_string()
first_value))._to_placeholder_string()
elif first_key == 'ifPresent':
structure_kwargs = arg['ifPresent']
@ -615,16 +615,16 @@ def maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
e, component_dict=component_dict)
for e in structure_kwargs['otherwise']
]
return IfPresentPlaceholder(**structure_kwargs).to_placeholder_string()
return IfPresentPlaceholder(**structure_kwargs)._to_placeholder_string()
elif first_key == 'concat':
return ConcatPlaceholder(items=[
maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
e, component_dict=component_dict) for e in arg['concat']
]).to_placeholder_string()
])._to_placeholder_string()
elif first_key == 'executorInput':
return ExecutorInputPlaceholder().to_placeholder_string()
return ExecutorInputPlaceholder()._to_placeholder_string()
elif 'if' in arg:
if_ = arg['if']
@ -640,13 +640,13 @@ def maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
else_=[
maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
val, component_dict=component_dict) for val in else_
]).to_placeholder_string()
])._to_placeholder_string()
elif 'concat' in arg:
return ConcatPlaceholder(items=[
maybe_convert_v1_yaml_placeholder_to_v2_placeholder_str(
val, component_dict=component_dict) for val in arg['concat']
]).to_placeholder_string()
])._to_placeholder_string()
else:
raise TypeError(f'Unexpected argument {arg} of type {type(arg)}.')

View File

@ -28,9 +28,9 @@ class TestExecutorInputPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.ExecutorInputPlaceholder):
self.assertEqual(
placeholders.ExecutorInputPlaceholder.from_placeholder_string(
placeholders.ExecutorInputPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -44,9 +44,9 @@ class TestInputValuePlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.InputValuePlaceholder):
self.assertEqual(
placeholders.InputValuePlaceholder.from_placeholder_string(
placeholders.InputValuePlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@parameterized.parameters([
@ -59,7 +59,7 @@ class TestInputValuePlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.InputValuePlaceholder):
self.assertEqual(
placeholders.InputValuePlaceholder.from_placeholder_string(
placeholders.InputValuePlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
@ -73,9 +73,9 @@ class TestInputPathPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.InputPathPlaceholder):
self.assertEqual(
placeholders.InputPathPlaceholder.from_placeholder_string(
placeholders.InputPathPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -89,9 +89,9 @@ class TestInputUriPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.InputUriPlaceholder):
self.assertEqual(
placeholders.InputUriPlaceholder.from_placeholder_string(
placeholders.InputUriPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -105,9 +105,9 @@ class TestInputMetadataPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.InputMetadataPlaceholder):
self.assertEqual(
placeholders.InputMetadataPlaceholder.from_placeholder_string(
placeholders.InputMetadataPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -121,9 +121,9 @@ class TestOutputPathPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.OutputPathPlaceholder):
self.assertEqual(
placeholders.OutputPathPlaceholder.from_placeholder_string(
placeholders.OutputPathPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -137,9 +137,9 @@ class TestOutputParameterPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.OutputParameterPlaceholder):
self.assertEqual(
placeholders.OutputParameterPlaceholder.from_placeholder_string(
placeholders.OutputParameterPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -153,9 +153,9 @@ class TestOutputUriPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.OutputUriPlaceholder):
self.assertEqual(
placeholders.OutputUriPlaceholder.from_placeholder_string(
placeholders.OutputUriPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -169,9 +169,9 @@ class TestOutputMetadataPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.OutputMetadataPlaceholder):
self.assertEqual(
placeholders.OutputMetadataPlaceholder.from_placeholder_string(
placeholders.OutputMetadataPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -211,9 +211,9 @@ class TestIfPresentPlaceholderStructure(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.ConcatPlaceholder):
self.assertEqual(
placeholders.IfPresentPlaceholder.from_placeholder_string(
placeholders.IfPresentPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@ -238,9 +238,9 @@ class TestConcatPlaceholder(parameterized.TestCase):
self, placeholder_string: str,
placeholder_obj: placeholders.ConcatPlaceholder):
self.assertEqual(
placeholders.ConcatPlaceholder.from_placeholder_string(
placeholders.ConcatPlaceholder._from_placeholder_string(
placeholder_string), placeholder_obj)
self.assertEqual(placeholder_obj.to_placeholder_string(),
self.assertEqual(placeholder_obj._to_placeholder_string(),
placeholder_string)
@parameterized.parameters([
@ -252,7 +252,7 @@ class TestConcatPlaceholder(parameterized.TestCase):
"some value{{$.inputs.parameters[''input_infix'']}}some value"
])
def test_is_match(self, placeholder: str):
self.assertTrue(placeholders.ConcatPlaceholder.is_match(placeholder))
self.assertTrue(placeholders.ConcatPlaceholder._is_match(placeholder))
@parameterized.parameters([
("{{$.inputs.parameters[''input1'']}}something{{$.inputs.parameters[''input2'']}}",
@ -277,8 +277,8 @@ class TestConcatPlaceholder(parameterized.TestCase):
def test_split_cel_concat_string(self, placeholder: str,
expected: List[str]):
self.assertEqual(
placeholders.ConcatPlaceholder.split_cel_concat_string(placeholder),
expected)
placeholders.ConcatPlaceholder._split_cel_concat_string(
placeholder), expected)
class TestProcessCommandArg(unittest.TestCase):

View File

@ -121,7 +121,7 @@ class InputSpec(InputSpec_, base_model.BaseModel):
else:
return False
def validate_type(self) -> None:
def _validate_type(self) -> None:
"""Type should either be a parameter or a valid bundled artifact type
by the time it gets to InputSpec.
@ -188,7 +188,7 @@ class OutputSpec(base_model.BaseModel):
else:
return False
def validate_type(self):
def _validate_type(self):
"""Type should either be a parameter or a valid bundled artifact type
by the time it gets to OutputSpec.
@ -276,15 +276,15 @@ class ContainerSpecImplementation(base_model.BaseModel):
resources: Optional[ResourceSpec] = None
"""Specification on the resource requirements."""
def transform_command(self) -> None:
def _transform_command(self) -> None:
"""Use None instead of empty list for command."""
self.command = None if self.command == [] else self.command
def transform_args(self) -> None:
def _transform_args(self) -> None:
"""Use None instead of empty list for args."""
self.args = None if self.args == [] else self.args
def transform_env(self) -> None:
def _transform_env(self) -> None:
"""Use None instead of empty dict for env."""
self.env = None if self.env == {} else self.env
@ -549,19 +549,19 @@ class ComponentSpec(base_model.BaseModel):
inputs: Optional[Dict[str, InputSpec]] = None
outputs: Optional[Dict[str, OutputSpec]] = None
def transform_name(self) -> None:
def _transform_name(self) -> None:
"""Converts the name to a valid name."""
self.name = utils.maybe_rename_for_k8s(self.name)
def transform_inputs(self) -> None:
def _transform_inputs(self) -> None:
"""Use None instead of empty list for inputs."""
self.inputs = None if self.inputs == {} else self.inputs
def transform_outputs(self) -> None:
def _transform_outputs(self) -> None:
"""Use None instead of empty list for outputs."""
self.outputs = None if self.outputs == {} else self.outputs
def validate_placeholders(self):
def _validate_placeholders(self):
"""Validates that input/output placeholders refer to an existing
input/output."""
if self.implementation.container is None: