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

Add mount args to some utils components #787

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

schmidt-ai
Copy link
Contributor

Closes #786.

Test plan:
TBD — do we have a good path for testing mounts?

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 2, 2023
@schmidt-ai schmidt-ai changed the title add mount args Add mount args to some utils components Nov 2, 2023
@kiukchung
Copy link
Collaborator

kiukchung commented Nov 2, 2023

Test plan: TBD — do we have a good path for testing mounts?

For components we typically test that the resulting AppDef is what we expect it to be. As an example for mounts take a look at https://github.com/pytorch/torchx/blob/main/torchx/components/test/dist_test.py#L17.

Functionally, since we reduce components to an AppDef, the tests for each scheduler implementation tests for whether mounts are respected correctly when we convert the AppDef into the respective scheduler job definitions. Here's an example with the kubernetes scheduler: https://github.com/pytorch/torchx/blob/main/torchx/schedulers/test/kubernetes_scheduler_test.py#L144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add mounts argument to more utils components
3 participants