From 871edab12536dee5967c52b8ed81a36af37e86b7 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Wed, 20 Feb 2019 14:10:03 -0800 Subject: [PATCH 1/5] extract the pipelineparam deserialize function --- sdk/python/kfp/dsl/_container_op.py | 8 +++----- sdk/python/kfp/dsl/_pipeline_param.py | 10 ++++++++++ sdk/python/tests/dsl/pipeline_param_tests.py | 12 ++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 90e27854807..7a6ded71247 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -15,6 +15,7 @@ from . import _pipeline from . import _pipeline_param +from ._pipeline_param import _extract_pipeleineparam import re from typing import Dict @@ -64,13 +65,10 @@ def __init__(self, name: str, image: str, command: str=None, arguments: str=None self.pod_labels = {} self.num_retries = 0 - matches = [] + self.argument_inputs = [] for arg in (command or []) + (arguments or []): - match = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?)}}', str(arg)) - matches += match + self.argument_inputs += _extract_pipeleineparam(str(arg)) - self.argument_inputs = [_pipeline_param.PipelineParam(x[1], x[0], x[2]) - for x in list(set(matches))] self.file_outputs = file_outputs self.dependent_op_names = [] diff --git a/sdk/python/kfp/dsl/_pipeline_param.py b/sdk/python/kfp/dsl/_pipeline_param.py index 213f9d10d73..17a09802376 100644 --- a/sdk/python/kfp/dsl/_pipeline_param.py +++ b/sdk/python/kfp/dsl/_pipeline_param.py @@ -21,6 +21,16 @@ # For now, this identifies a condition with only "==" operator supported. ConditionOperator = namedtuple('ConditionOperator', 'operator operand1 operand2') +def _extract_pipeleineparam(payload: str): + """_extract_pipelineparam extract a list of PipelineParam instances from the payload string. + + Args: + payload (str): a string that contains serialized pipelineparams + Return: + List[PipelineParam] + """ + matches = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?)}}', payload) + return [PipelineParam(x[1], x[0], x[2]) for x in list(set(matches))] class PipelineParam(object): """Representing a future value that is passed between pipeline components. diff --git a/sdk/python/tests/dsl/pipeline_param_tests.py b/sdk/python/tests/dsl/pipeline_param_tests.py index 7cd2fd7d687..82641b5572f 100644 --- a/sdk/python/tests/dsl/pipeline_param_tests.py +++ b/sdk/python/tests/dsl/pipeline_param_tests.py @@ -14,6 +14,7 @@ from kfp.dsl import PipelineParam +from kfp.dsl._pipeline_param import _extract_pipeleineparam import unittest @@ -35,3 +36,14 @@ def test_str_repr(self): p = PipelineParam(name='param3', value='value3') self.assertEqual('{{pipelineparam:op=;name=param3;value=value3}}', str(p)) + + def test_extract_pipelineparam(self): + """Test _extract_pipeleineparam.""" + + p1 = PipelineParam(name='param1', op_name='op1') + p2 = PipelineParam(name='param2') + p3 = PipelineParam(name='param3', value='value3') + stuff_chars = ' between ' + payload = str(p1) + stuff_chars + str(p2) + stuff_chars + str(p3) + params = _extract_pipeleineparam(payload) + self.assertListEqual([p1, p2, p3], params) \ No newline at end of file From 5d697f9efe8764a4e090daa1c8deb9a8e033356d Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Thu, 21 Feb 2019 13:12:50 -0800 Subject: [PATCH 2/5] typo fix --- sdk/python/kfp/dsl/_container_op.py | 4 ++-- sdk/python/kfp/dsl/_pipeline_param.py | 2 +- sdk/python/tests/dsl/pipeline_param_tests.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 7a6ded71247..727965f3eab 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -15,7 +15,7 @@ from . import _pipeline from . import _pipeline_param -from ._pipeline_param import _extract_pipeleineparam +from ._pipeline_param import _extract_pipelineparam import re from typing import Dict @@ -67,7 +67,7 @@ def __init__(self, name: str, image: str, command: str=None, arguments: str=None self.argument_inputs = [] for arg in (command or []) + (arguments or []): - self.argument_inputs += _extract_pipeleineparam(str(arg)) + self.argument_inputs += _extract_pipelineparam(str(arg)) self.file_outputs = file_outputs self.dependent_op_names = [] diff --git a/sdk/python/kfp/dsl/_pipeline_param.py b/sdk/python/kfp/dsl/_pipeline_param.py index 17a09802376..dbd77db67c6 100644 --- a/sdk/python/kfp/dsl/_pipeline_param.py +++ b/sdk/python/kfp/dsl/_pipeline_param.py @@ -21,7 +21,7 @@ # For now, this identifies a condition with only "==" operator supported. ConditionOperator = namedtuple('ConditionOperator', 'operator operand1 operand2') -def _extract_pipeleineparam(payload: str): +def _extract_pipelineparam(payload: str): """_extract_pipelineparam extract a list of PipelineParam instances from the payload string. Args: diff --git a/sdk/python/tests/dsl/pipeline_param_tests.py b/sdk/python/tests/dsl/pipeline_param_tests.py index 82641b5572f..2da3b0fc451 100644 --- a/sdk/python/tests/dsl/pipeline_param_tests.py +++ b/sdk/python/tests/dsl/pipeline_param_tests.py @@ -14,7 +14,7 @@ from kfp.dsl import PipelineParam -from kfp.dsl._pipeline_param import _extract_pipeleineparam +from kfp.dsl._pipeline_param import _extract_pipelineparam import unittest @@ -45,5 +45,5 @@ def test_extract_pipelineparam(self): p3 = PipelineParam(name='param3', value='value3') stuff_chars = ' between ' payload = str(p1) + stuff_chars + str(p2) + stuff_chars + str(p3) - params = _extract_pipeleineparam(payload) + params = _extract_pipelineparam(payload) self.assertListEqual([p1, p2, p3], params) \ No newline at end of file From a7802ebc87d9584926b88c53493efe1f066915a5 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 22 Feb 2019 11:47:08 -0800 Subject: [PATCH 3/5] adjust extract_param to accept a list of strings --- sdk/python/kfp/dsl/_container_op.py | 6 ++---- sdk/python/kfp/dsl/_pipeline_param.py | 11 ++++++++--- sdk/python/tests/dsl/pipeline_param_tests.py | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 727965f3eab..20dead080a4 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -15,7 +15,7 @@ from . import _pipeline from . import _pipeline_param -from ._pipeline_param import _extract_pipelineparam +from ._pipeline_param import _extract_pipelineparams import re from typing import Dict @@ -65,9 +65,7 @@ def __init__(self, name: str, image: str, command: str=None, arguments: str=None self.pod_labels = {} self.num_retries = 0 - self.argument_inputs = [] - for arg in (command or []) + (arguments or []): - self.argument_inputs += _extract_pipelineparam(str(arg)) + self.argument_inputs = _extract_pipelineparams((command or []) + (arguments or [])) self.file_outputs = file_outputs self.dependent_op_names = [] diff --git a/sdk/python/kfp/dsl/_pipeline_param.py b/sdk/python/kfp/dsl/_pipeline_param.py index dbd77db67c6..2b5a5db7e8c 100644 --- a/sdk/python/kfp/dsl/_pipeline_param.py +++ b/sdk/python/kfp/dsl/_pipeline_param.py @@ -21,15 +21,20 @@ # For now, this identifies a condition with only "==" operator supported. ConditionOperator = namedtuple('ConditionOperator', 'operator operand1 operand2') -def _extract_pipelineparam(payload: str): +def _extract_pipelineparams(payloads: str or list[str]): """_extract_pipelineparam extract a list of PipelineParam instances from the payload string. + Note: this function removes all duplicate matches. Args: - payload (str): a string that contains serialized pipelineparams + payload (str or list[str]): a string/a list of strings that contains serialized pipelineparams Return: List[PipelineParam] """ - matches = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?)}}', payload) + if isinstance(payloads, str): + payloads = [payloads] + matches = [] + for payload in payloads: + matches = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?)}}', payload) return [PipelineParam(x[1], x[0], x[2]) for x in list(set(matches))] class PipelineParam(object): diff --git a/sdk/python/tests/dsl/pipeline_param_tests.py b/sdk/python/tests/dsl/pipeline_param_tests.py index 2da3b0fc451..580531cbda1 100644 --- a/sdk/python/tests/dsl/pipeline_param_tests.py +++ b/sdk/python/tests/dsl/pipeline_param_tests.py @@ -14,7 +14,7 @@ from kfp.dsl import PipelineParam -from kfp.dsl._pipeline_param import _extract_pipelineparam +from kfp.dsl._pipeline_param import _extract_pipelineparams import unittest @@ -45,5 +45,5 @@ def test_extract_pipelineparam(self): p3 = PipelineParam(name='param3', value='value3') stuff_chars = ' between ' payload = str(p1) + stuff_chars + str(p2) + stuff_chars + str(p3) - params = _extract_pipelineparam(payload) + params = _extract_pipelineparams(payload) self.assertListEqual([p1, p2, p3], params) \ No newline at end of file From c1726563ecf06f0207712c454fa55d07b1b99c10 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 22 Feb 2019 12:00:57 -0800 Subject: [PATCH 4/5] convert arg to string --- sdk/python/kfp/dsl/_container_op.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 20dead080a4..75a88405784 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -65,7 +65,7 @@ def __init__(self, name: str, image: str, command: str=None, arguments: str=None self.pod_labels = {} self.num_retries = 0 - self.argument_inputs = _extract_pipelineparams((command or []) + (arguments or [])) + self.argument_inputs = _extract_pipelineparams([str(arg) for arg in (command or []) + (arguments or [])]) self.file_outputs = file_outputs self.dependent_op_names = [] From a96fe06fb036c4d1c0a13de347a43b19e308dd1c Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 22 Feb 2019 17:19:31 -0800 Subject: [PATCH 5/5] bug fix, add unit tests --- sdk/python/kfp/dsl/_pipeline_param.py | 2 +- sdk/python/tests/dsl/pipeline_param_tests.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/python/kfp/dsl/_pipeline_param.py b/sdk/python/kfp/dsl/_pipeline_param.py index 2b5a5db7e8c..a2c0d67714c 100644 --- a/sdk/python/kfp/dsl/_pipeline_param.py +++ b/sdk/python/kfp/dsl/_pipeline_param.py @@ -34,7 +34,7 @@ def _extract_pipelineparams(payloads: str or list[str]): payloads = [payloads] matches = [] for payload in payloads: - matches = re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?)}}', payload) + matches += re.findall(r'{{pipelineparam:op=([\w\s_-]*);name=([\w\s_-]+);value=(.*?)}}', payload) return [PipelineParam(x[1], x[0], x[2]) for x in list(set(matches))] class PipelineParam(object): diff --git a/sdk/python/tests/dsl/pipeline_param_tests.py b/sdk/python/tests/dsl/pipeline_param_tests.py index 580531cbda1..ed403aa6516 100644 --- a/sdk/python/tests/dsl/pipeline_param_tests.py +++ b/sdk/python/tests/dsl/pipeline_param_tests.py @@ -46,4 +46,7 @@ def test_extract_pipelineparam(self): stuff_chars = ' between ' payload = str(p1) + stuff_chars + str(p2) + stuff_chars + str(p3) params = _extract_pipelineparams(payload) + self.assertListEqual([p1, p2, p3], params) + payload = [str(p1) + stuff_chars + str(p2), str(p2) + stuff_chars + str(p3)] + params = _extract_pipelineparams(payload) self.assertListEqual([p1, p2, p3], params) \ No newline at end of file