Skip to content

Conversation

aivanou
Copy link
Contributor

@aivanou aivanou commented Oct 14, 2021

…ainer

Test plan:

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@aivanou has updated the pull request. You must reimport the pull request before landing.

@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 14, 2021
@codecov
Copy link

codecov bot commented Oct 14, 2021

Codecov Report

Merging #255 (45eb637) into main (fcf19f8) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #255      +/-   ##
==========================================
+ Coverage   91.56%   91.61%   +0.05%     
==========================================
  Files          60       60              
  Lines        2762     2767       +5     
==========================================
+ Hits         2529     2535       +6     
+ Misses        233      232       -1     
Impacted Files Coverage Δ
torchx/components/interpret.py 100.00% <ø> (ø)
torchx/runner/api.py 96.29% <ø> (-0.06%) ⬇️
torchx/specs/__init__.py 88.88% <ø> (ø)
torchx/cli/cmd_run.py 89.89% <100.00%> (+0.79%) ⬆️
torchx/runner/config.py 100.00% <100.00%> (ø)
torchx/specs/api.py 99.28% <100.00%> (+<0.01%) ⬆️
torchx/specs/finder.py 97.36% <100.00%> (+0.05%) ⬆️

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 fcf19f8...45eb637. Read the comment docs.

@facebook-github-bot
Copy link
Contributor

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

Kiuk Chung and others added 3 commits October 14, 2021 12:14
… runcfg (#252)

Summary:
Pull Request resolved: #252

closes: #250

I've done a few things on this diff:

1. Renamed `torchx.specs.api.Runopt` to `torchx.specs.runopt` (for consistency with `runopts`)
2. Renamed variables (where I could) `runcfg` to `cfg` (to be consistent with the scheduler and runner apis)
3. Renamed the config section to `[$profile.$sched.cfg]`  instead of `[$profile.scheduler_args.$sched]`
4. Changed the torchx run cli's `-a` (short for `--scheduler_args`) to `-cfg` for consistency with the rest of the system.

Reviewed By: aivanou

Differential Revision: D31656766

fbshipit-source-id: 8c009852d5807010ac4cd33902b294cff4bd0ec1
Summary: closes: #251

Reviewed By: d4l3k

Differential Revision: D31659813

fbshipit-source-id: 150ea152339adf84d19f1eb8b4a2e083901705ab
@facebook-github-bot
Copy link
Contributor

@aivanou has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

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

d4l3k and others added 10 commits October 14, 2021 19:25
Summary:
This switches our integration tests to use the GitHub OpenID Connect credentials provider instead of using hard coded AWS session tokens. This will issue tokens that last for 1 hour so should be a lot more secure (and trackable) than before.

https://awsteele.com/blog/2021/09/15/aws-federation-comes-to-github-actions.html

Pull Request resolved: #256

Test Plan:
CI

created PR from external repo to verify they can't generate tokens #257

Reviewed By: kiukchung

Differential Revision: D31674489

Pulled By: d4l3k

fbshipit-source-id: 5936c64794816eb9fafe76899af44e2f865c64df
Summary: Since we removed distributed sum, we need to use this example to run fb internal tests. For internal tests, we don't need the `output_path`, which introduces around ~200 mb of data on each run

Reviewed By: kiukchung

Differential Revision: D31661378

fbshipit-source-id: 098bf9f5be9302e7d8cced672ba9cf7eaf8b32e6
Summary:
Pull Request resolved: #258

Do not check `torchx.components.base` module for components

Reviewed By: kiukchung

Differential Revision: D31664832

fbshipit-source-id: 3de72047810ff8c2478e036ce5626459d4c073af
…al loading, and move config loading to cli from runner (#260)

Summary:
Pull Request resolved: #260

1. Removes profiles from .torchxconfig (also removes .cfg suffix from section)
2. Removes hierarchical loading (only picks up .torchxconfig from CWD - project dir)
3. Removes config application from runner and moves it to CLI only

Reviewed By: d4l3k

Differential Revision: D31674537

fbshipit-source-id: 937c3375771316b2bf2f1d65a560d7311031d4fa
Summary:
This adds the missing `pandoc` dependency to the docpush CI step.

Pull Request resolved: #264

Test Plan:
Test with docpush manually enabled on the PR
https://github.com/pytorch/torchx/pull/264/checks?check_run_id=3908761230

Reviewed By: aivanou

Differential Revision: D31692193

Pulled By: d4l3k

fbshipit-source-id: 0fcb9b5667ec096d458d4e293c0cd1b34d402f7d
…265)

Summary:
This switches the integration tests to use ec2 instance connect w/ an assumed role instead of embedding the slurm ssh key in GitHub secrets.

Pull Request resolved: #265

Test Plan:
```
$ env SLURM_INSTANCE_MASTER=ubuntu@i-01dd4b95724eb0b4b scripts/slurmint.sh
```

CI

Reviewed By: kiukchung, aivanou

Differential Revision: D31695261

Pulled By: d4l3k

fbshipit-source-id: 48a52e911e68bc9b18ed470a5f7e725ff58697b1
@facebook-github-bot
Copy link
Contributor

@aivanou has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@aivanou merged this pull request in d277d8c.

@d4l3k d4l3k deleted the aivanou_add_interpret_docs branch October 21, 2021 16:40
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. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants