Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make pod_spec_patch more accessible #818

Open
flaviuvadan opened this issue Oct 26, 2023 · 2 comments
Open

Make pod_spec_patch more accessible #818

flaviuvadan opened this issue Oct 26, 2023 · 2 comments
Labels
type:enhancement A general enhancement

Comments

@flaviuvadan
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
It is incredibly cumbersome to write out a pod spec patch specification. One can do it either as a plain string ('{"containers":[{"name":"main", "resources":{"limits":{"cpu": "{{workflow.parameters.cpu-limit}}" }}}]}') or via json.dumps({'containers': [{'name': 'main', ...}]}). json.dumps is nicer because users don't have to spend time fiddling with quotes but the whole concept is challenging to grasp.

Describe the solution you'd like
A way for Hera to set up these dynamic pod spec patches for the user. A way for script to know when to patch and when to not patch perhaps.

Describe alternatives you've considered
Not doing it at all and going with status quo

@flaviuvadan flaviuvadan added the type:enhancement A general enhancement label Oct 26, 2023
@elliotgunton
Copy link
Collaborator

Considering the actual spec has

    pod_spec_patch: Optional[str] = Field(
        default=None,
        alias="podSpecPatch",
        description=(
            "PodSpecPatch holds strategic merge patch to apply against the pod spec. Allows parameterization of"
            " container fields which are not strings (e.g. resource limits)."
        ),
    )

I would suggest a Hera pydantic class with a build function, and add a union type with it for Workflows

pod_spec_patch: Annotated[Optional[str], _WorkflowModelMapper("spec.pod_spec_patch")] = None

and templates

pod_spec_patch: Optional[str] = None

💯

@flaviuvadan
Copy link
Collaborator Author

@elliotgunton I assume you're suggesting a style of parsing a pod spec patch rather than setting the pod spec patch on the workflow right? pod spec patching is set on a template level. If that's the case I think I agree - Hera needs a way to map some lighter pod spec patch specification to the actual pod spec patch string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants