(torchx/scheduler)(aws) add instance type for aws_batch_scheduler multinode jobs

[clumsy]

add instance type for aws_batch_scheduler multinode jobs

Test plan:

• added two unit tests: one for singlenode and one for multinode job

[codecov]

Codecov Report

Merging #781 (09ba107) into main (a711634) will decrease coverage by 0.01%.
Report is 1 commits behind head on main.
The diff coverage is 97.36%.

@@            Coverage Diff             @@
##             main     #781      +/-   ##
==========================================
- Coverage   92.88%   92.88%   -0.01%     
==========================================
  Files          96       96              
  Lines        6099     6112      +13     
==========================================
+ Hits         5665     5677      +12     
- Misses        434      435       +1     

Flag	Coverage Δ
unittests	92.88% <97.36%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
torchx/schedulers/aws_batch_scheduler.py	90.44% <100.00%> (+0.13%)	⬆️
torchx/specs/named_resources_aws.py	98.50% <97.05%> (-1.50%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[clumsy]

@kiukchung please find an updated commit based on our discussion.

[clumsy]

btw lint failure seems unrelated to my change, @kiukchung

[schmidt-ai]

Excited for this!

[kiukchung]

@clumsy let me know if you're blocked on anything here.

[clumsy]

@kiukchung just curious if you agree it's better to introduce an AWS resource subclass to provide (self-documented) instance_type attribute instead of relying on K8S_TYPE behavior that needs to be documented separately. Asking because I'd like to use instance_type for my upcoming SageMaker scheduler contribution.
Basically If you're OK with the current PR - I'll address the failures and submit a new version.

[kiukchung]

@kiukchung just curious if you agree it's better to introduce an AWS resource subclass to provide (self-documented) instance_type attribute instead of relying on K8S_TYPE behavior that needs to be documented separately. Asking because I'd like to use instance_type for my upcoming SageMaker scheduler contribution. Basically If you're OK with the current PR - I'll address the failures and submit a new version.

I'd like to avoid introducing subclasses for Resource to keep the specs API easier to serialize (e.g. to JSON, YAML, Proto). We can use K8S_TYPE (and document that this will be used by the batch and SM schedulers to set instance type params in the respective scheduler's request).

Edit: what would be useful is to have a util method that both batch and sm schedulers use that returns the aws instance type given a Resource (it would pull this info from K8S_TYPE key from the capabilities map). This method would also throw an error with a useful message when K8S_TYPE is not found, perhaps prompting the user to add one to their resource specs for self-service fix.

[clumsy]

@kiukchung I've made the changes as discussed, the docs build failure does not seem to be caused by my changes.

[kiukchung]

LGTM thanks