argoproj-labs / hera

Hera makes Python code easy to orchestrate on Argo Workflows through native Python integrations. It lets you construct and submit your Workflows entirely in Python. ⭐️ Remember to star!
https://hera.rtfd.io
Apache License 2.0
593 stars 106 forks source link

Upgrading to 5.10 causes validation error #879

Closed vikramsg closed 11 months ago

vikramsg commented 11 months ago

Pre-bug-report checklist

1. This bug can be reproduced using pure Argo YAML

If yes, it is more likely to be an Argo bug unrelated to Hera. Please double check before submitting an issue to Hera.

2. This bug occurs in Hera when...

Bug report

Describe the bug I have a workflow that worked fine with Hera 5.9 but stopped working with 5.10. The errors shown are not very helpful in terms of trying to debug.

  File "/home/vikramsg/Projects/pipeline/pipe-src/orchestrator/example/simple_parallel/dag.py", line 41, in <module>
    workflow.create()
  File "/home/vikramsg/Projects/pipeline/.venv/lib/python3.8/site-packages/hera/workflows/workflow.py", line 385, in create
    wf = self.workflows_service.create_workflow(
  File "/home/vikramsg/Projects/pipeline/.venv/lib/python3.8/site-packages/hera/workflows/service.py", line 849, in create_workflow
    return Workflow(**resp.json())
  File "pydantic/main.py", line 341, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 3 validation errors for Workflow
metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)

To Reproduce Full Hera code to reproduce the bug:

from typing import List

from hera.workflows import script
from pydantic import BaseModel

from orchestrator.example.simple import config

class Sample(BaseModel):
    file_id: int
    file: str
    metadata: str

class EchoSampleInput(BaseModel):
    file: str
    metadata: str

@script()
def transform_parameters(files: List[str]) -> List[Sample]:
    return [Sample(file_id=it, file=file, metadata="read") for it, file in enumerate(files)]

@script()
def transform_input(sample: Sample) -> EchoSampleInput:
    return EchoSampleInput(file=sample.file, metadata=sample.metadata)

@script()
def file_output(echo_input: EchoSampleInput) -> str:
    return echo_input.file

from hera.workflows import Parameter, Steps, Workflow

from orchestrator.example.simple.config import set_server_config
from orchestrator.example.simple_parallel.business import file_output, transform_input
from orchestrator.example.simple_parallel.business import transform_parameters

def create_workflow() -> Workflow:
    set_server_config()

    with Workflow(
        generate_name="pipleine-",
        entrypoint="pipeline-steps",
        arguments=[Parameter(name="logs-directory", value="/tmp/logs")],
    ) as w:
        # Implementation step for a single sample.
        # Given a sample, do a transform and then output the file. It expects a variable of type Sample.
        with Steps(name="transform-steps", inputs=Parameter(name="sample")) as transform_step:
            sample_transform_step = transform_input(arguments=Parameter(name="sample", value="{{inputs.parameters.sample}}"))
            file_output(arguments=sample_transform_step.get_result_as("echo_input"))

        # Driver step.
        # First do a step that creates a list of samples, and then run a parallel pod for each sample.
        with Steps(name="pipeline-steps"):
            transform_parameters_step = transform_parameters(
                arguments=Parameter(name="files", value=["checksum1.txt", "checksum2.txt", "checksum3.txt"])
            )
            # The transform_step is a callable that expects an input with name sample.
            # with_param takes in a list and then launches a pod with each member of the list.
            # The {{item}} syntax is Argo's way of telling what each member of the list is.
            transform_step(
                with_param=transform_parameters_step.get_result_as("sample"), arguments=Parameter(name="sample", value="{{item}}")
            )

    return w

if __name__ == "__main__":
    workflow = create_workflow()
    workflow.create()

Expected behavior Workflow should just work as it did for 5.9.

Environment

elliotgunton commented 11 months ago

Seems like the reverse of https://github.com/argoproj-labs/hera/issues/636 at first glance.

This issue is because we added https://github.com/argoproj-labs/hera/blob/206b51a8fffcad3fda7e2f042a27c1451f10cf25/src/hera/shared/_base_model.py#L30-L31

Eylidian commented 11 months ago

It may be related, I have the same issue but it's just when requesting the list of CRON workflows:

service = WorkflowsService(host=configuration.ARGO_SERVICE_URI, token=configuration.ARGO_SERVICE_API_KEY)
cron_workflows = service.list_cron_workflows(namespace=configuration.ENVIRONMENT_NAME)

StackTrace:

  cron_workflows = service.list_cron_workflows(namespace=configuration.ENVIRONMENT_NAME)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "d:\usr\venv\Lib\site-packages\hera\workflows\service.py", line 417, in list_cron_workflows
    return CronWorkflowList(**resp.json())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "pydantic\main.py", line 341, in pydantic.main.BaseModel.__init__       
pydantic.error_wrappers.ValidationError: 63 validation errors for CronWorkflowList
items -> 0 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 0 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 0 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 0 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 1 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 1 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 1 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 2 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 2 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 2 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 3 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 3 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 3 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 4 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 4 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 4 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 5 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 5 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 5 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 6 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 6 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 6 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 7 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 7 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 7 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 8 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 8 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 8 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 9 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata
  extra fields not permitted (type=value_error.extra)
items -> 9 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 9 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 10 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 10 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 10 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 11 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 11 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 11 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 12 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 12 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 12 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 13 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 13 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 13 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 13 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 14 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 14 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 14 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 14 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 15 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 15 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 15 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 15 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 16 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 16 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 16 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 17 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 17 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 17 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 17 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)
items -> 18 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 18 -> metadata -> managedFields -> 0 -> fieldsV1 -> f:spec
  extra fields not permitted (type=value_error.extra)
items -> 18 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:metadata        
  extra fields not permitted (type=value_error.extra)
items -> 18 -> metadata -> managedFields -> 1 -> fieldsV1 -> f:status
  extra fields not permitted (type=value_error.extra)  

With 5.9 worked fine.

elliotgunton commented 11 months ago

CC @flaviuvadan maybe we should prioritise #652 to solve this in future? πŸ˜…

sambhav commented 11 months ago

I think I would prefer to revert this change temporarily and cut a patch release ASAP.

flaviuvadan commented 11 months ago

https://github.com/argoproj-labs/hera/pull/880

sambhav commented 11 months ago

Fixed with 5.10.1