Skip to content

Commit

Permalink
SDK - Cleanup - Serialized PipelineParamTuple does not need value or…
Browse files Browse the repository at this point in the history
… type (kubeflow#1469)

* SDK - Refactoring - Serialized PipelineParam does not need type
Only the types in non-serialized PipelineParams are ever used.

* SDK - Refactoring - Serialized PipelineParam does not need value
Default values are only relevant when PipelineParam is used in the pipeline function signature and even in this case compiler captures them explicitly from the pipelineParam objects in the signature.
There is no other uses for them.
  • Loading branch information
Ark-kun authored and k8s-ci-robot committed Aug 16, 2019
1 parent d2e94e4 commit 54ff3e6
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 27 deletions.
27 changes: 4 additions & 23 deletions sdk/python/kfp/dsl/_pipeline_param.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
# TODO: Move this to a separate class
# For now, this identifies a condition with only "==" operator supported.
ConditionOperator = namedtuple('ConditionOperator', 'operator operand1 operand2')
PipelineParamTuple = namedtuple('PipelineParamTuple', 'name op value type pattern')
PipelineParamTuple = namedtuple('PipelineParamTuple', 'name op pattern')


def sanitize_k8s_name(name):
Expand All @@ -40,26 +40,13 @@ def match_serialized_pipelineparam(payload: str):
Returns:
PipelineParamTuple
"""
matches = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?);type=(.*?);}}', payload)
if len(matches) == 0:
matches = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?)}}', payload)
matches = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+)}}', payload)
param_tuples = []
for match in matches:
if len(match) == 3:
pattern = '{{pipelineparam:op=%s;name=%s;value=%s}}' % (match[0], match[1], match[2])
pattern = '{{pipelineparam:op=%s;name=%s}}' % (match[0], match[1])
param_tuples.append(PipelineParamTuple(
name=sanitize_k8s_name(match[1]),
op=sanitize_k8s_name(match[0]),
value=match[2],
type='',
pattern=pattern))
elif len(match) == 4:
pattern = '{{pipelineparam:op=%s;name=%s;value=%s;type=%s;}}' % (match[0], match[1], match[2], match[3])
param_tuples.append(PipelineParamTuple(
name=sanitize_k8s_name(match[1]),
op=sanitize_k8s_name(match[0]),
value=match[2],
type=match[3],
pattern=pattern))
return param_tuples

Expand All @@ -81,8 +68,6 @@ def _extract_pipelineparams(payloads: str or List[str]):
for param_tuple in list(set(param_tuples)):
pipeline_params.append(PipelineParam(param_tuple.name,
param_tuple.op,
param_tuple.value,
TypeMeta.deserialize(param_tuple.type),
pattern=param_tuple.pattern))
return pipeline_params

Expand Down Expand Up @@ -205,11 +190,7 @@ def __str__(self):
# return str(self.value)

op_name = self.op_name if self.op_name else ''
value = self.value if self.value else ''
if self.param_type is None:
return '{{pipelineparam:op=%s;name=%s;value=%s}}' % (op_name, self.name, value)
else:
return '{{pipelineparam:op=%s;name=%s;value=%s;type=%s;}}' % (op_name, self.name, value, self.param_type.serialize())
return '{{pipelineparam:op=%s;name=%s}}' % (op_name, self.name)

def __repr__(self):
return str({self.__class__.__name__: self.__dict__})
Expand Down
2 changes: 1 addition & 1 deletion sdk/python/tests/compiler/compiler_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def test_operator_to_template(self):
'inputs': {'parameters':
[
{'name': 'msg1'},
{'name': 'msg2', 'value': 'value2'},
{'name': 'msg2'},
]},
'name': 'echo',
'outputs': {
Expand Down
6 changes: 3 additions & 3 deletions sdk/python/tests/dsl/pipeline_param_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ def test_str_repr(self):
"""Test string representation."""

p = PipelineParam(name='param1', op_name='op1')
self.assertEqual('{{pipelineparam:op=op1;name=param1;value=;type=;}}', str(p))
self.assertEqual('{{pipelineparam:op=op1;name=param1}}', str(p))

p = PipelineParam(name='param2')
self.assertEqual('{{pipelineparam:op=;name=param2;value=;type=;}}', str(p))
self.assertEqual('{{pipelineparam:op=;name=param2}}', str(p))

p = PipelineParam(name='param3', value='value3')
self.assertEqual('{{pipelineparam:op=;name=param3;value=value3;type=;}}', str(p))
self.assertEqual('{{pipelineparam:op=;name=param3}}', str(p))

def test_extract_pipelineparams(self):
"""Test _extract_pipeleineparams."""
Expand Down

0 comments on commit 54ff3e6

Please sign in to comment.