Skip to content

Conversation

@kiukchung
Copy link
Contributor

Summary:
NOTE: not ready to be officially reviewed yet - publishing early in case someone's curious.

Part 1/2 (need an additional diff to make changes in penv python)

Does the following:

  1. Implements a JetterWorkspaceScheduler that uses jetter to build patches. Falls back to torchx custom patch building for BC.

  2. Removes the now unnecessary fb/cmd_run (since we moved everything to workspaces.

  3. Makes mast and flow schedulers a subclass of JetterWorkspaceScheduler

Differential Revision: D33971673

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Feb 3, 2022
@facebook-github-bot
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented Feb 3, 2022

Codecov Report

Merging #380 (c299c92) into main (8b62ea8) will decrease coverage by 0.84%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #380      +/-   ##
==========================================
- Coverage   93.96%   93.12%   -0.85%     
==========================================
  Files          64       66       +2     
  Lines        3565     3535      -30     
==========================================
- Hits         3350     3292      -58     
- Misses        215      243      +28     
Impacted Files Coverage Δ
torchx/workspace/docker_workspace.py 35.89% <35.89%> (ø)
torchx/workspace/api.py 83.33% <83.33%> (ø)
torchx/cli/cmd_run.py 89.70% <100.00%> (-1.14%) ⬇️
torchx/runner/api.py 96.57% <100.00%> (+0.25%) ⬆️
torchx/schedulers/api.py 92.64% <100.00%> (+2.50%) ⬆️
torchx/schedulers/docker_scheduler.py 95.62% <100.00%> (-1.59%) ⬇️
torchx/schedulers/kubernetes_scheduler.py 89.76% <100.00%> (-0.10%) ⬇️
torchx/schedulers/local_scheduler.py 93.25% <100.00%> (ø)
torchx/workspace/__init__.py 100.00% <100.00%> (ø)
torchx/runner/events/__init__.py 94.44% <0.00%> (-2.78%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b62ea8...c299c92. Read the comment docs.

@facebook-github-bot
Copy link
Contributor

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

6 similar comments
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

…lement jetter_workspace_scheduler (#380)

Summary:
Pull Request resolved: #380

~~Part 1/2 (need an additional diff to make changes in penv python)~~ (done with this diff)

Does the following:

1. Adds a `torchx.workspace.Workspace` MixIn for Schedulers. Move Docker workspace build from docker_scheduler to torchx.workspace.DockerWorkspace

1. Implements a `JetterWorkspace` that uses jetter to build patches. Falls back to torchx custom patch building for BC.

1. Removes the now unnecessary fb/cmd_run (since we moved everything to workspaces.

1. Adds the `JetterWorkspace` mixin to mast, flow, qf, local_fb schedulers

1. Takes out `DockerWorkspace` from docker_scheduler.build_workspace_and_update_role()

1. Adds the `DockerWorkspace` mixin to docker and k8s schedulers

1. Gets rid of `WorkspaceScheduler` (in favor of `Workspace` mixin) and `WorkspaceRunner` (just builds an optional workspace parameter Runner APIs)

1. (bugfix) read from the config_dirs (home + cwd) to get component default args

Reviewed By: d4l3k

Differential Revision: D33971673

fbshipit-source-id: 3a97553b840c3b60aae651cd1815a667f29e770e
@facebook-github-bot
Copy link
Contributor

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

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. fb-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants