Skip to content

Commit

Permalink
add default value type checking (#1407)
Browse files Browse the repository at this point in the history
* add default value type checking

* add jsonschema dependency

* fix unit test error

* workaround for travis python package installation

* add back jsonschema version

* fix sample test error in type checking sample

* add jsonschema in requirements such that sphinx works fine
  • Loading branch information
gaoning777 authored and k8s-ci-robot committed Jun 3, 2019
1 parent 895645c commit b6967d8
Show file tree
Hide file tree
Showing 11 changed files with 150 additions and 67 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ matrix:
- language: python
python: "3.6"
env: TOXENV=py36
install: pip3 install jsonschema==3.0.1
script:
# DSL tests
- cd $TRAVIS_BUILD_DIR/sdk/python
Expand Down
39 changes: 24 additions & 15 deletions samples/notebooks/DSL Static Type Checking.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
},
{
"cell_type": "code",
"execution_count": 3,
"execution_count": 1,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -255,7 +255,7 @@
},
{
"cell_type": "code",
"execution_count": 4,
"execution_count": 2,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -393,7 +393,7 @@
},
{
"cell_type": "code",
"execution_count": 7,
"execution_count": 5,
"metadata": {},
"outputs": [],
"source": [
Expand All @@ -417,7 +417,7 @@
},
{
"cell_type": "code",
"execution_count": 1,
"execution_count": 6,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -477,7 +477,7 @@
},
{
"cell_type": "code",
"execution_count": 9,
"execution_count": 7,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -507,7 +507,7 @@
},
{
"cell_type": "code",
"execution_count": 5,
"execution_count": 9,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -565,7 +565,7 @@
},
{
"cell_type": "code",
"execution_count": 11,
"execution_count": 10,
"metadata": {},
"outputs": [
{
Expand Down Expand Up @@ -600,7 +600,7 @@
},
{
"cell_type": "code",
"execution_count": 12,
"execution_count": 11,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -630,7 +630,7 @@
},
{
"cell_type": "code",
"execution_count": 6,
"execution_count": 12,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -687,7 +687,7 @@
},
{
"cell_type": "code",
"execution_count": 14,
"execution_count": 13,
"metadata": {},
"outputs": [],
"source": [
Expand All @@ -710,7 +710,7 @@
},
{
"cell_type": "code",
"execution_count": 15,
"execution_count": 14,
"metadata": {},
"outputs": [],
"source": [
Expand All @@ -733,9 +733,18 @@
},
{
"cell_type": "code",
"execution_count": 6,
"execution_count": 15,
"metadata": {},
"outputs": [],
"outputs": [
{
"name": "stdout",
"output_type": "stream",
"text": [
"Integer has a property openapi_schema_validator that the latter does not.\n",
"Component \"task_factory_a\" is expecting field_o to be type(Integer), but the passed argument is type({'Integer': {'openapi_schema_validator': {'type': 'integer'}}})\n"
]
}
],
"source": [
"@component\n",
"def task_factory_a(field_m: {'GCSPath': {'openapi_schema_validator': '{\"type\": \"string\", \"pattern\": \"^gs://.*$\"}'}}, field_o: 'Integer'):\n",
Expand All @@ -751,7 +760,7 @@
"# Pipeline input types are also checked against the component I/O types.\n",
"@dsl.pipeline(name='type_check_g',\n",
" description='')\n",
"def pipeline_g(a: {'GCSPath': {'openapi_schema_validator': '{\"type\": \"string\", \"pattern\": \"^gs://.*$\"}'}}='good', b: Integer()=12):\n",
"def pipeline_g(a: {'GCSPath': {'openapi_schema_validator': '{\"type\": \"string\", \"pattern\": \"^gs://.*$\"}'}}='gs://kfp-path', b: Integer()=12):\n",
" task_factory_a(field_m=a, field_o=b)\n",
"\n",
"try:\n",
Expand All @@ -769,7 +778,7 @@
},
{
"cell_type": "code",
"execution_count": 17,
"execution_count": 16,
"metadata": {},
"outputs": [],
"source": [
Expand Down
31 changes: 21 additions & 10 deletions samples/notebooks/Lightweight Python components - basics.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 1,
"metadata": {},
"outputs": [],
"source": [
Expand All @@ -61,7 +61,7 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 2,
"metadata": {},
"outputs": [],
"source": [
Expand All @@ -80,7 +80,7 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 3,
"metadata": {},
"outputs": [],
"source": [
Expand All @@ -96,7 +96,7 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 4,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -154,9 +154,20 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 9,
"metadata": {},
"outputs": [],
"outputs": [
{
"data": {
"text/plain": [
"MyDivmodOutput(quotient=14, remainder=2)"
]
},
"execution_count": 9,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"my_divmod(100, 7)"
]
Expand All @@ -172,7 +183,7 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 6,
"metadata": {},
"outputs": [],
"source": [
Expand All @@ -189,7 +200,7 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 7,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -223,7 +234,7 @@
},
{
"cell_type": "code",
"execution_count": null,
"execution_count": 8,
"metadata": {},
"outputs": [],
"source": [
Expand Down Expand Up @@ -278,7 +289,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.5.4rc1"
"version": "3.6.5"
}
},
"nbformat": 4,
Expand Down
16 changes: 15 additions & 1 deletion sdk/python/kfp/dsl/_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ def serialize(self):

@staticmethod
def deserialize(payload):
# If the payload is a string of a dict serialization, convert it back to a dict
'''deserialize expects two types of input: dict and str
1) If the payload is a string of a dict serialization, convert it back to a dict
2) If the payload is a string, the type is named as such with no properties.
3) If the payload is a dict, the type name and properties are extracted. '''
try:
import ast
payload = ast.literal_eval(payload)
Expand Down Expand Up @@ -138,6 +141,9 @@ def _annotation_to_typemeta(annotation):
'''_annotation_to_type_meta converts an annotation to an instance of TypeMeta
Args:
annotation(BaseType/str/dict): input/output annotations
BaseType: registered in kfp.dsl.types
str: either a string of a dict serialization or a string of the type name
dict: type name and properties. note that the properties values can be dict.
Returns:
TypeMeta
'''
Expand Down Expand Up @@ -233,6 +239,14 @@ def _extract_pipeline_metadata(func):
arg_default = arg_default.value
if arg in annotations:
arg_type = _annotation_to_typemeta(annotations[arg])
if 'openapi_schema_validator' in arg_type.properties and arg_default is not None:
from jsonschema import validate
import json
schema_object = arg_type.properties['openapi_schema_validator']
if isinstance(schema_object, str):
# In case the property value for the schema validator is a string instead of a dict.
schema_object = json.loads(arg_type.properties['openapi_schema_validator'])
validate(instance=arg_default, schema=schema_object)
pipeline_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type, default=arg_default))

#TODO: add descriptions to the metadata
Expand Down
81 changes: 46 additions & 35 deletions sdk/python/kfp/dsl/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,65 +19,76 @@ class BaseType:

# Primitive Types
class Integer(BaseType):
openapi_schema_validator = {
"type": "integer"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "integer"
}

class String(BaseType):
openapi_schema_validator = {
"type": "string"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "string"
}

class Float(BaseType):
openapi_schema_validator = {
"type": "number"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "number"
}

class Bool(BaseType):
openapi_schema_validator = {
"type": "boolean"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "boolean"
}

class List(BaseType):
openapi_schema_validator = {
"type": "array"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "array"
}

class Dict(BaseType):
openapi_schema_validator = {
"type": "object",
}
def __init__(self):
self.openapi_schema_validator = {
"type": "object",
}

# GCP Types
class GCSPath(BaseType):
openapi_schema_validator = {
"type": "string",
"pattern": "^gs://.*$"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "string",
"pattern": "^gs://.*$"
}

class GCRPath(BaseType):
openapi_schema_validator = {
"type": "string",
"pattern": "^.*gcr\\.io/.*$"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "string",
"pattern": "^.*gcr\\.io/.*$"
}

class GCPRegion(BaseType):
openapi_schema_validator = {
"type": "string"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "string"
}

class GCPProjectID(BaseType):
'''MetaGCPProjectID: GCP project id'''
openapi_schema_validator = {
"type": "string"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "string"
}

# General Types
class LocalPath(BaseType):
#TODO: add restriction to path
openapi_schema_validator = {
"type": "string"
}
def __init__(self):
self.openapi_schema_validator = {
"type": "string"
}

class InconsistentTypeException(Exception):
'''InconsistencyTypeException is raised when two types are not consistent'''
Expand Down
1 change: 1 addition & 0 deletions sdk/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ google-auth>=1.6.1
requests_toolbelt>=0.8.0
kfp-server-api >= 0.1.18, < 0.1.19
argo-models == 2.2.1a
jsonschema >= 3.0.1
Loading

0 comments on commit b6967d8

Please sign in to comment.