Skip to content

Conversation

d4l3k
Copy link
Member

@d4l3k d4l3k commented Oct 17, 2022

This makes a few changes intended to make it easier to swap out the workspace implementation for schedulers that support multiple image runtimes. I.e. LSF and Slurm support Docker, Singularity, native, etc

  • This adds a new workspace_opts so we can keep runopts attached to the workspace
  • This standardizes the dryrun_push_images and push_images that were used in DockerWorkspace so we can swap the workspace without running into any type issues.

Test plan:

Refactor -- no functional changes. Existing tests/CI should suffice

@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 Oct 17, 2022
@facebook-github-bot
Copy link
Contributor

@d4l3k has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@codecov
Copy link

codecov bot commented Oct 17, 2022

Codecov Report

Merging #619 (e72dd5b) into main (da19c5f) will decrease coverage by 0.02%.
The diff coverage is 96.92%.

@@            Coverage Diff             @@
##             main     #619      +/-   ##
==========================================
- Coverage   94.97%   94.95%   -0.03%     
==========================================
  Files          68       68              
  Lines        4659     4676      +17     
==========================================
+ Hits         4425     4440      +15     
- Misses        234      236       +2     
Impacted Files Coverage Δ
torchx/workspace/api.py 92.53% <83.33%> (-2.30%) ⬇️
torchx/runner/api.py 94.89% <100.00%> (ø)
torchx/schedulers/api.py 94.33% <100.00%> (+0.28%) ⬆️
torchx/schedulers/aws_batch_scheduler.py 88.93% <100.00%> (-0.09%) ⬇️
torchx/schedulers/docker_scheduler.py 95.97% <100.00%> (-0.03%) ⬇️
torchx/schedulers/kubernetes_scheduler.py 93.38% <100.00%> (-0.06%) ⬇️
torchx/schedulers/local_scheduler.py 93.12% <100.00%> (ø)
torchx/schedulers/lsf_scheduler.py 99.61% <100.00%> (ø)
torchx/schedulers/ray_scheduler.py 95.29% <100.00%> (ø)
torchx/schedulers/slurm_scheduler.py 97.00% <100.00%> (ø)
... and 4 more

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

… push_images (#619)

Summary:
This makes a few changes intended to make it easier to swap out the workspace implementation for schedulers that support multiple image runtimes. I.e. LSF and Slurm support Docker, Singularity, native, etc

* This adds a new `workspace_opts` so we can keep runopts attached to the workspace
* This standardizes the `dryrun_push_images` and `push_images` that were used in DockerWorkspace so we can swap the workspace without running into any type issues.

Pull Request resolved: #619

Test Plan: Refactor -- no functional changes. Existing tests/CI should suffice

Reviewed By: kurman

Differential Revision: D40446676

Pulled By: d4l3k

fbshipit-source-id: 0e43b159c558991f228bba808f35b9393508d67c
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40446676

@d4l3k d4l3k deleted the workspaceopts branch October 28, 2022 20:24
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.

2 participants