Skip to content

Commit

Permalink
fix(sdk.v2): Fix passing in "" to a str parameter causes the parame…
Browse files Browse the repository at this point in the history
…ter to receive it as None instead (#6533)

* fix empty string not passed through

* update release note
  • Loading branch information
chensun authored Sep 9, 2021
1 parent b6cb0b4 commit 38e826b
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 48 deletions.
1 change: 1 addition & 0 deletions sdk/RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

* Remove dead code on importer check in v1. [\#6508](https://github.com/kubeflow/pipelines/pull/6508)
* Fix issue where dict, list, bool typed input parameters don't accept constant values or pipeline inputs. [\#6523](https://github.com/kubeflow/pipelines/pull/6523)
* Fix passing in "" to a str parameter causes the parameter to receive it as None instead. [\#6533](https://github.com/kubeflow/pipelines/pull/6533)
* Depends on `kfp-pipeline-spec>=0.1.10,<0.2.0` [\#6515](https://github.com/kubeflow/pipelines/pull/6515)

## Documentation Updates
Expand Down
4 changes: 2 additions & 2 deletions sdk/python/kfp/v2/components/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import json
import inspect
import json
from typing import Any, Callable, Dict, List, Optional, Union

from kfp.v2.components.types import artifact_types, type_annotations
Expand Down Expand Up @@ -70,7 +70,7 @@ def _get_input_parameter_value(self, parameter_name: str,
if parameter is None:
return None

if parameter.get('stringValue'):
if parameter.get('stringValue') is not None:
if parameter_type == str:
return parameter['stringValue']
elif parameter_type == bool:
Expand Down
66 changes: 20 additions & 46 deletions sdk/python/kfp/v2/components/executor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,18 @@
# limitations under the License.
"""Tests for kfp.components.executor."""

import json
import os
import tempfile
from typing import Callable, NamedTuple, Optional
import unittest
import json
from typing import Callable, NamedTuple, Optional

from kfp.v2.components import executor
from kfp.v2.components.types import artifact_types
from kfp.v2.components.types.artifact_types import Artifact, Dataset, Model, Metrics
from kfp.v2.components.types.type_annotations import Input, Output, InputPath, OutputPath
from kfp.v2.components.types.artifact_types import (Artifact, Dataset, Metrics,
Model)
from kfp.v2.components.types.type_annotations import (Input, InputPath, Output,
OutputPath)

_EXECUTOR_INPUT = """\
{
Expand Down Expand Up @@ -220,6 +222,9 @@ def test_function_string_output(self):
"stringValue": "Hello"
},
"second_message": {
"stringValue": ""
},
"third_message": {
"stringValue": "World"
}
}
Expand All @@ -235,8 +240,12 @@ def test_function_string_output(self):
}
"""

def test_func(first_message: str, second_message: str) -> str:
return first_message + ", " + second_message
def test_func(
first_message: str,
second_message: str,
third_message: str,
) -> str:
return first_message + ", " + second_message + ", " + third_message

self._get_executor(test_func, executor_input).execute()
with open(os.path.join(self._test_dir, 'output_metadata.json'),
Expand All @@ -245,7 +254,7 @@ def test_func(first_message: str, second_message: str) -> str:
self.assertDictEqual(output_metadata, {
"parameters": {
"Output": {
"stringValue": "Hello, World"
"stringValue": "Hello, , World"
}
},
})
Expand Down Expand Up @@ -289,45 +298,6 @@ def test_func(first: int, second: int) -> int:
},
})

def test_function_with_int_output(self):
executor_input = """\
{
"inputs": {
"parameters": {
"first_message": {
"stringValue": "Hello"
},
"second_message": {
"stringValue": "World"
}
}
},
"outputs": {
"artifacts": {
"Output": {
"outputFile": "gs://some-bucket/output"
}
},
"outputFile": "%s/output_metadata.json"
}
}
"""

def test_func(first_message: str, second_message: str) -> str:
return first_message + ", " + second_message

self._get_executor(test_func, executor_input).execute()
with open(os.path.join(self._test_dir, 'output_metadata.json'),
'r') as f:
output_metadata = json.loads(f.read())
self.assertDictEqual(output_metadata, {
"parameters": {
"Output": {
"stringValue": "Hello, World"
}
},
})

def test_artifact_output(self):
executor_input = """\
{
Expand Down Expand Up @@ -467,3 +437,7 @@ def func_returning_plain_tuple() -> NamedTuple('Outputs', [
'r') as f:
artifact_payload = f.read()
self.assertEqual(artifact_payload, "Dataset contents")


if __name__ == '__main__':
unittest.main()

0 comments on commit 38e826b

Please sign in to comment.