Resource Limits defaults

[Trollgeir]

I've noticed how Resource limits - if not defined - default to the minimum requested. This was a gotcha for me as I expected there to be no limits in a resource if I didn't explicitly defined it.

Also, is there a reason why the terminology differs form kubernetes (limit vs maximum)? This is contradictory to the declaration in "About" for this project:

...while still maintaining a consistent vocabulary with Argo Workflows.

[flaviuvadan]

Resource limits - if not defined - default to the minimum requested. This was a gotcha for me as I expected there to be no limits in a resource if I didn't explicitly defined it

I can definitely see how this behavior can be confusing and warrants some adjustments. I think we can safely remove the limit as there are cases, like yours, in which users do not want to set an explicit limit

reason why the terminology differs form kubernetes

Hera aimed to increase access to Argo Workflows for the scientific/DS community at the start. min vs max are terms that are more broadly accessible

contradictory to the declaration in "About" for this project:

I see how this manifests as a contradiction but that statement refers to how Hera aims to maintain consistency in vocabulary with Argo, not necessarily Kubernetes. This refers to "workflow", "task", etc, which are higher level entities by comparison to the resource limits set for Kubernetes. Again, I definitely see your argument and perhaps there's space for improvement.

How about one of the following

Resources(cpu=Tuple[int, int], mem=Tuple[str, str], gpu=int)  # assumes that "nvidia.com/gpu" is the correct selector for GPUs, I prefer this because it's explicit, maybe add an optional field for "GPU" field with a "nvidia.com/gpu" default

Resources(requests: Dict[str, Union[int, str]], limits=Optional[Dict[str, Union[int, str]]])  # arbitrary resource setting, might be hard for users who are not familiar with requests/limits of K8S but this is by far the closest one can get to K8S and allows the removal of the `custom` resources field

Resources(cpu_req: int, mem_req: int, cpu_limit: Optional[int], mem_req: int, mem_limit: Optional[int], ...)  # very verbose and requires a lot of maintenance probably

[Trollgeir]

If a developer has worked with kubernetes or Argo Workflows before, it would be a seamless transition to construct the Resources object as the arguments are the same. If a DS-developer (without background in the former) created a new Resources object, I would assume they don't have a preconception over how pod resources are generally defined, unless they're coming from a custom service which had its own API deciding to use different terminology (in other words, non-standard).

I think a DS wouldn't have a strong preference for either of these implementations, but I think a developer with background in Kubernetes/Argo workflows would (but I could be wrong!). Also, a DS would inadvertently learn a lot of kubernetes-terminology when they first start to glance at manifests for the first time in a different context; "Oh, I actually understand what this yml does!". :)

 Resources(requests: Dict[str, Union[int, str]], limits=Optional[Dict[str, Union[int, str]]])  # arbitrary resource setting, might be hard for users who are not familiar with requests/limits of K8S but this is by far the closest one can get to K8S and allows the removal of the `custom` resources field

Although perhaps a bit complicated for new users, this is currently the only solution that supports new arbitrary types of limits/requests. Here's an example of a semi-common limit that we currently do not support: ephemeral-storage: "4Gi". One simple solution would be to add that as an additional optional argument, but I would prefer keeping the args arbitrary so this requires less maintenance on our side in case new types show up.

How about instead we make some new helper-objects which inherits from Resources, like SimpleResources and GPUResources with some custom arguments you described above?