argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
15.12k stars 3.21k forks source link

Discrepancy Between Generated Python OpenAPI Client And Argo Server #4510

Closed kennytrytek-wf closed 3 years ago

kennytrytek-wf commented 4 years ago

Summary

I can make a request using the HTTP API that fails when using the Python client generated from the swagger.json.

Running Argo server locally, I navigate to this URL in my browser: http://localhost:2746/api/v1/workflows/argo?fields=items.metadata.name I have a few example workflows, so the response looks like this:

{
  "items": [
    {
      "metadata": {
        "name": "example-mlcwl"
      }
    },
    {
      "metadata": {
        "name": "example-7slfm"
      }
    },
    {
      "metadata": {
        "name": "example-s86zz"
      }
    }
  ]
}

Generating the Python OpenAPI client:

docker run --rm \
      -v ${PWD}/argo-openapi-client:/local openapitools/openapi-generator-cli generate \
      -i /local/swagger.json \
      -g python \
      -o /local \
      --additional-properties=packageName=argo,projectName=argo

And using it to make the same request:

>>> import argo
>>> configuration = argo.Configuration(host='http://localhost:2746')
>>> with argo.ApiClient(configuration=configuration) as api_client:
...   api_instance = argo.WorkflowServiceApi(api_client)
...

>>> api_instance.list_workflows('argo', fields='items.metadata.name')
Traceback (most recent call last):
<snip>
ValueError: Invalid value for `spec`, must not be `None`

>>> api_instance.list_workflows('argo', fields='items.metadata.name,items.spec.affinity')
Traceback (most recent call last):
<snip>
ValueError: Invalid value for `metadata`, must not be `None`

>>> api_instance.list_workflows('argo', fields='items.metadata.name,items.spec.affinity,metadata')
{'api_version': None,
 'items': [{'api_version': None,
            'kind': None,
            'metadata': {'annotations': None,
...

Looking at the swagger.json, I can see that those fields are required, but I'm confused about how the server fulfills the same request in the browser if the request doesn't meet the requirements of the spec, and whether the generated client should match that behavior?

$ argo version
argo: latest+a00a8f1.dirty
  BuildDate: 2020-10-08T01:44:49Z
  GitCommit: a00a8f141c221f50e397aea8f86a54171441e395
  GitTreeState: dirty
  GitTag: v2.11.3
  GoVersion: go1.15.2
  Compiler: gc
  Platform: darwin/amd64

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

alexec commented 4 years ago

Can you ask in the #argo-sdks channel?

kennytrytek-wf commented 4 years ago

Can you ask in the #argo-sdks channel?

https://argoproj.slack.com/archives/CT5HFHDK8/p1605050218009600

kennytrytek-wf commented 4 years ago

While continuing to look at this, I tried the following (which did not work):

config = argo.Configuration(host="http://localhost:2746")
config.client_side_validation = False
with argo.ApiClient(configuration=config) as api_client:
    api_instance = argo.WorkflowServiceApi(api_client)
    results = api_instance.list_workflows( "argo", fields="items.metadata.name")

I noticed that this client_side_validation variable exists on the Configuration object and is always set to True. That same variable is referenced in a few places in WorkflowServiceApi like this: self.api_client.client_side_validation. It also has a corresponding idea in the generated model files, e.g. io_argoproj_workflow_v1alpha1_workflow.py:

class IoArgoprojWorkflowV1alpha1Workflow(object):
    def __init__(self, api_version=None, kind=None, metadata=None, spec=None, status=None, local_vars_configuration=None):  # noqa: E501
        """IoArgoprojWorkflowV1alpha1Workflow - a model defined in OpenAPI"""  # noqa: E501
        if local_vars_configuration is None:
            local_vars_configuration = Configuration()
        self.local_vars_configuration = local_vars_configuration

    @spec.setter
    def spec(self, spec):
        """Sets the spec of this IoArgoprojWorkflowV1alpha1Workflow.
        :param spec: The spec of this IoArgoprojWorkflowV1alpha1Workflow.  # noqa: E501
        :type spec: IoArgoprojWorkflowV1alpha1WorkflowSpec
        """
        if self.local_vars_configuration.client_side_validation and spec is None:  # noqa: E501
            raise ValueError("Invalid value for `spec`, must not be `None`")  # noqa: E501

        self._spec = spec

If the client_side_validation setting in the WorkflowServiceApi could propagate to where the models are instantiated after the request returns, then this wouldn't be a problem. As it is, the model instantiation behavior is inconsistent with the Configuration used in the request and raises the ValueError no matter what because local_vars_configuration is never populated.

yxue-kabam commented 4 years ago

@kennytrytek-wf try out https://pypi.org/project/argo-workflows ?

yxue-kabam commented 4 years ago

OK this is the argo server api correct (not submitting the workflow), this is on my todo list for above project (add an issue there? maybe we can work together on this)

kennytrytek-wf commented 4 years ago

OK this is the argo server api correct

Yes, this is the argo server API as it compares to the generated OpenAPI client. I know the argo-client-python project exists, but I want to use the client as generated from the OpenAPI spec to ensure it is always compatible with the version of Argo server we have deployed.

My question is more fundamental about how the OpenAPI spec and Argo server relate to each other; should the server return responses that aren't supported by the spec (i.e. the example above where the response objects are missing required attributes), and should the OpenAPI client relax its definitions or add support for those queries?

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lloyd-dotmodus commented 3 years ago

@kennytrytek-wf @yxue-kabam I've come across this exact same issue trying to list cron workflows. I'm using the latest 5.0.0 version of the API (https://pypi.org/project/argo-workflows/)

Is there any work being done to fix this issue? Or at leat the ability to set client_side_validation to False as alluded to above?

Regards

lloyd-dotmodus commented 3 years ago

I've added a new issue here: https://github.com/argoproj/argo-workflows/issues/6399

nikunjmasrani commented 3 years ago

I am facing the same issue. does anyone have a solution?

terrytangyuan commented 3 years ago

Can you try the new SDK (note that it's still experimental and not published to PyPI yet): https://github.com/argoproj/argo-workflows/tree/master/sdks/python