Hi team! Can anyone take a look at my PR? It adds the disk parameter to the @resources decorator. Thanks!

thanks for the PR @proud-forest-76332! one gotcha here is that

@resources

supports both

@batch

and

@kubernetes

- unfortunately,

disk

isn't support in

@batch

- that's the reason why it is missing in

@resources

.

Thanks @square-wire-39606 for the feedback! Would it make sense to add an environment variable instead so we can set the default value of the disk parameter on Kubernetes?

@bulky-afternoon-92433 ^^

@proud-forest-76332 I think adding an environment variable for the default is unnecessary at this point, unless you already have a use case for setting custom defaults in mind? None of the other args for the resource decorator have env configurable defaults either, so it would be a bit of a departure from the implementation. Adding the docstring that I posted should suffice and we can go forward with the PR. There's also another aspect to consider regarding defaults on

@resources

vs

@kubernetes

, as the current implementation tries to pick the larger of the two attributes for each attr specified in both. Specifying any value for the resource deco that is smaller than a default for the kubernetes decorator will simply get overwritten. So for example

@resources(disk=4096)

will still yield a disk of 10240M

Thanks @bulky-afternoon-92433! No need for the environment variable if the changes for the @resources decorator can go through. I have updated the PR to include the docstring as well.