fix(yaml): V2D-1504 Failure in Deserializing Successful Response When Creating SeldonDeployment with SDK

[ChewieSC]

This was done because the current swagger yaml expects IntOrString (which is the type of pdbSpec from the original ticket) to be a dictionary and not either an int or a string....so the correct definition according to the yaml would be this:

      pdbSpec:
        IntVal: 1
        Type: Integer

This, however, does not correctly translate to the eventual Core deployment.

So we introduced a patch to fix the type in the SDK implementation.

To note down what the swagger yaml should look like, here is the diff of the yaml:

  IntOrString:
       +protobuf=true
       +protobuf.options.(gogoproto.goproto_stringer)=false
       +k8s:openapi-gen=true
-    properties:
-      IntVal:
+    oneOf:
+      - type: integer
         format: int32
-        type: integer
-      StrVal:
-        type: string
-      Type:
-        $ref: '#/definitions/Type'
+      - type: string
     title: |-
       IntOrString is a type that can hold an int32 or a string.  When used in
       JSON or YAML marshalling and unmarshalling, it produces or consumes the
       inner type.  This allows you to have, for example, a JSON field that can
       accept a name or number.
       TODO: Rename to Int32OrString
-    type: object

I did not commit this since it will have no inherent value for the actual SDK.

As I understand now, oneOf is part of the OpenAPI v3 specification.
That means we could also use something like https://github.com/getkin/kin-openapi in Deploy to make this patch obsolete. But with the many, many moving parts atm, that might not be the best idea (for now).

EDIT:
Just as a reference because it could be helpful in the future (Michael found this): kubernetes-client/gen#93.
Here they're moving to https://github.com/openapitools/openapi-generator.

[jesse-c]

I think that instead of editing this type itself, the pdbSpec type should be edited, to use the correct type?

[jesse-c]

question: I don't think this should be deleted, since elsewhere we still use the IntOrString type? [1]

[1]

$ rg IntOrString                                                                                                                       
swagger-v1alpha1.yml
2505:        $ref: '#/definitions/IntOrString'
2657:  IntOrString:
2671:      IntOrString is a type that can hold an int32 or a string.  When used in
6056:        $ref: '#/definitions/IntOrString'
6058:        $ref: '#/definitions/IntOrString'
6560:        $ref: '#/definitions/IntOrString'
6562:        $ref: '#/definitions/IntOrString'
7008:        $ref: '#/definitions/IntOrString'
7230:        $ref: '#/definitions/IntOrString'
7312:    title: Type represents the stored type of IntOrString.

python/README.md
319: - [IntOrString](docs/IntOrString.md)

python/docs/HTTPGetAction.md
9:**port** | [**IntOrString**](IntOrString.md) |  | [optional]

python/docs/ServicePort.md
11:**target_port** | [**IntOrString**](IntOrString.md) |  | [optional]

python/docs/TCPSocketAction.md
7:**port** | [**IntOrString**](IntOrString.md) |  | [optional]

python/docs/SeldonPdbSpec.md
6:**max_unavailable** | [**IntOrString**](IntOrString.md) |  | [optional]
7:**min_available** | [**IntOrString**](IntOrString.md) |  | [optional]

python/docs/RollingUpdateDeployment.md
6:**max_surge** | [**IntOrString**](IntOrString.md) |  | [optional]
7:**max_unavailable** | [**IntOrString**](IntOrString.md) |  | [optional]

python/test/test_int_or_string.py
19:from seldon_deploy_sdk.models.int_or_string import IntOrString  # noqa: E501
23:class TestIntOrString(unittest.TestCase):
24:    """IntOrString unit test stubs"""
32:    def testIntOrString(self):
33:        """Test IntOrString"""
35:        # model = seldon_deploy_sdk.models.int_or_string.IntOrString()  # noqa: E501

python/docs/IntOrString.md
1:# IntOrString

python/seldon_deploy_sdk/__init__.py
156:from seldon_deploy_sdk.models.int_or_string import IntOrString

python/seldon_deploy_sdk/models/rolling_update_deployment.py
34:        'max_surge': 'IntOrString',
35:        'max_unavailable': 'IntOrString'
61:        :rtype: IntOrString
71:        :type: IntOrString
82:        :rtype: IntOrString
92:        :type: IntOrString

python/seldon_deploy_sdk/models/seldon_pdb_spec.py
34:        'max_unavailable': 'IntOrString',
35:        'min_available': 'IntOrString'
61:        :rtype: IntOrString
71:        :type: IntOrString
82:        :rtype: IntOrString
92:        :type: IntOrString

python/seldon_deploy_sdk/models/tcp_socket_action.py
35:        'port': 'IntOrString'
84:        :rtype: IntOrString
94:        :type: IntOrString

python/seldon_deploy_sdk/models/service_port.py
39:        'target_port': 'IntOrString'
194:        :rtype: IntOrString
204:        :type: IntOrString

python/seldon_deploy_sdk/models/__init__.py
128:from seldon_deploy_sdk.models.int_or_string import IntOrString

python/seldon_deploy_sdk/models/int_or_string.py
20:class IntOrString(object):
46:        """IntOrString - a model defined in Swagger"""  # noqa: E501
62:        """Gets the int_val of this IntOrString.  # noqa: E501
65:        :return: The int_val of this IntOrString.  # noqa: E501
72:        """Sets the int_val of this IntOrString.
75:        :param int_val: The int_val of this IntOrString.  # noqa: E501
83:        """Gets the str_val of this IntOrString.  # noqa: E501
86:        :return: The str_val of this IntOrString.  # noqa: E501
93:        """Sets the str_val of this IntOrString.
96:        :param str_val: The str_val of this IntOrString.  # noqa: E501
104:        """Gets the type of this IntOrString.  # noqa: E501
107:        :return: The type of this IntOrString.  # noqa: E501
114:        """Sets the type of this IntOrString.
117:        :param type: The type of this IntOrString.  # noqa: E501
144:        if issubclass(IntOrString, dict):
160:        if not isinstance(other, IntOrString):

python/seldon_deploy_sdk/models/http_get_action.py
37:        'port': 'IntOrString',
145:        :rtype: IntOrString
155:        :type: IntOrString

[ChewieSC]

Just as a note for why this works:
in __deserialize_model (api_client.py) there is this:

if (not klass.swagger_types and
        not self.__hasattr(klass, 'get_real_child_model')):
    return data

which is now true (no swagger_type no more), so we return data and thereby supply the intended base type (i.e. the int or string - which K8s expects).

Tested with int and string examples.

[michaelcheah]

Very nice. This seems to be the most sensible version until we move towards.

We don't have a set process for releasing this SDK yet, but I would suggest we do updating of the rc versions as separate PRs.

[michaelcheah]

Nice one