-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
fix(cloud): detect and ignore venv #16056
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! I've been adding venv
to my .lightningignore
for every new app I create.
if not (root / DOT_IGNORE_FILENAME).is_file(): | ||
with open(root / DOT_IGNORE_FILENAME, "w") as f: | ||
f.write("venv/\n") | ||
if (root / "bin" / "activate").is_file() or (root / "pyvenv.cfg").is_file(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could write the lines below without checking this. Do you expect any impact of having extra ignores?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check checks whether we are inside of venv and then ignores typical directories that exist in a venv dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get that. I meant if we should ignore them even if we aren't inside a venev
.
This is generally convenient. For example, just as GitHub creates a .gitignore
for you with a bunch of Python blobs from libraries even if you don't use any of them.
Do you expect any harm in that?
72a8d88
to
5d10181
Compare
Co-authored-by: Ethan Harris <[email protected]> (cherry picked from commit 3b323c8)
Co-authored-by: Ethan Harris <[email protected]> (cherry picked from commit 3b323c8)
* update chlog * CI: Add remote fetch (#16001) Co-authored-by: thomas <[email protected]> (cherry picked from commit 37fe3f6) * Set the logger explicitly in tests (#15815) (cherry picked from commit 9ed43c6) * [App] Fix `AutoScaler` trying to replicate multiple works in a single machine (#15991) * dont try to replicate new works in the existing machine * update chglog * Update comment * Update src/lightning_app/components/auto_scaler.py * add test (cherry picked from commit c1d0156) * Fix typo in PR titles generated by github-actions bot (#16003) (cherry picked from commit 2dcebc2) * Update docker requirement from <=5.0.3,>=5.0.0 to >=5.0.0,<6.0.2 in /requirements (#16007) Update docker requirement in /requirements Updates the requirements on [docker](https://github.com/docker/docker-py) to permit the latest version. - [Release notes](https://github.com/docker/docker-py/releases) - [Commits](docker/docker-py@5.0.0...6.0.1) --- updated-dependencies: - dependency-name: docker dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 4083b20) * Update deepdiff requirement from <=5.8.1,>=5.7.0 to >=5.7.0,<6.2.3 in /requirements (#16006) Update deepdiff requirement in /requirements Updates the requirements on [deepdiff](https://github.com/seperman/deepdiff) to permit the latest version. - [Release notes](https://github.com/seperman/deepdiff/releases) - [Changelog](https://github.com/seperman/deepdiff/blob/master/docs/changelog.rst) - [Commits](seperman/deepdiff@5.7.0...6.2.2) --- updated-dependencies: - dependency-name: deepdiff dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 5e705fa) * app: update doctest_skip (#15997) simple Co-authored-by: hhsecond <[email protected]> (cherry picked from commit 4fea6bf) * CI: clean install & share pkg build (#15986) * abstract pkg build * share ci * syntax * Checkgroup * folders * whl 1st * doctest Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Carlos Mocholí <[email protected]> (cherry picked from commit 18a4638) * Adding hint to the logger's error messages (#16034) Co-authored-by: Jirka Borovec <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> Fixes #15143 (cherry picked from commit 7ce3825) * fix publish * Introduce `{Work,Flow}.lightningignore` (#15818) (cherry picked from commit edd2b42) * [App] Support running on multiple clusters (#16016) (cherry picked from commit d3a7226) * [App] Improve lightning connect experience (#16035) (cherry picked from commit e522a12) * Cleanup cluster waiting (#16054) (cherry picked from commit 6458a5a) * feature(cli): login flow fixes and improvements (#16052) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit ebe7848) * Add guards to cluster deletion from cli (#16053) Adds guards to cluster deletion. - If cluster has running apps -> throw an error - If cluster has stopped apps -> confirm w/ user that apps and logs will be deleted (cherry picked from commit 64d0ebb) * Load app before setting LIGHTNING_DISPATCHED (#16057) (cherry picked from commit 8d3339a) * [App] Hot fix: Resolve detection of python debugger (#16068) Co-authored-by: thomas <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> (cherry picked from commit eae56ee) * fix(cloud): detect and ignore venv (#16056) Co-authored-by: Ethan Harris <[email protected]> (cherry picked from commit 3b323c8) * version 1.8.5 * update chlog Co-authored-by: thomas chaton <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: Akihiro Nitta <[email protected]> Co-authored-by: Adrian Wälchli <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Huy Đỗ <[email protected]> Co-authored-by: Ethan Harris <[email protected]> Co-authored-by: Luca Furst <[email protected]> Co-authored-by: Yurij Mikhalevich <[email protected]> Co-authored-by: Luca Antiga <[email protected]>
Co-authored-by: Ethan Harris <[email protected]>
* Remove the deprecated profiler imports (#16059) * Revert "Load app before setting LIGHTNING_DISPATCHED" (#16064) Revert "Load app before setting LIGHTNING_DISPATCHED (#16057)" This reverts commit 8d3339a. * [App] Hot fix: Resolve detection of python debugger (#16068) Co-authored-by: thomas <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> * Load the app before setting `LIGHTNING_DISPATCHED` (#16071) * fix(cloud): detect and ignore venv (#16056) Co-authored-by: Ethan Harris <[email protected]> * Add function to remove checkpoint to allow override for extended classes (#16067) * Drop FairScale sharded parity tests (#16069) * minor fix: indent spaces in comment-out (#16076) * ci: print existing candidates (#16077) * [App] Fix bug where previously deleted apps cannot be re-run from the CLI (#16082) * Better check for programmatic lightningignore (#16080) Co-authored-by: Jirka Borovec <[email protected]> * [App] Removing single quote (#16079) * [App] PoC: Add support for Request (#16047) * Have checkgroup pull the latest runs (#16033) * Update Multinode Warning (#16091) * [App] Serve datatypes with better client code (#16018) * docs: add PT version (#16010) * docs: add PT version * stable Co-authored-by: Adrian Wälchli <[email protected]> Co-authored-by: Adrian Wälchli <[email protected]> * add 1.13.1 to adjust versions (#16099) * Remove redundant `find_unused_parameters=False` in Lite (#16026) * [App] Add display name property to the work (#16095) Co-authored-by: thomas <[email protected]> * Fix detection of whether app is running in cloud (#16045) * [App] Add work.delete (#16103) Co-authored-by: thomas <[email protected]> * [App] Improve the autoscaler UI (#16063) [App] Improve the autoscaler UI (#16063) * Re-enable Lite CLI on Windows + PyTorch 1.13 (#15645) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Justus Schock <[email protected]> * [App] Min replica=0 would break autoscaler component (#16092) * fixing the bug where num_replica=0 would fail * changelog * [App] Scale out/in interval for autoscaler (#16093) * Adding arguments for scale out/in interval * Tests * Set the default work start method to spawn on MacOS (#16089) * [App] Add status endpoint, enable `ready` (#16075) Co-authored-by: thomas chaton <[email protected]> * Clarify `work.stop()` limitation (#16073) * fix merge errors * Update torchvision requirement from <=0.14.0,>=0.11.1 to >=0.11.1,<0.15.0 in /requirements (#16108) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]> * CI: settle file names (#16098) * CI: settle file names * rename * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Fix test failing on master due to bad auto-merge (#16118) * fix merge error Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: Akihiro Nitta <[email protected]> Co-authored-by: thomas chaton <[email protected]> Co-authored-by: thomas <[email protected]> Co-authored-by: Yurij Mikhalevich <[email protected]> Co-authored-by: Ethan Harris <[email protected]> Co-authored-by: Sean Naren <[email protected]> Co-authored-by: Qiushi Pan <[email protected]> Co-authored-by: Jirka Borovec <[email protected]> Co-authored-by: Sherin Thomas <[email protected]> Co-authored-by: Justus Schock <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]>
* Remove the deprecated profiler imports (#16059) * Revert "Load app before setting LIGHTNING_DISPATCHED" (#16064) Revert "Load app before setting LIGHTNING_DISPATCHED (#16057)" This reverts commit 8d3339a. * [App] Hot fix: Resolve detection of python debugger (#16068) Co-authored-by: thomas <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> * Load the app before setting `LIGHTNING_DISPATCHED` (#16071) * fix(cloud): detect and ignore venv (#16056) Co-authored-by: Ethan Harris <[email protected]> * Add function to remove checkpoint to allow override for extended classes (#16067) * Drop FairScale sharded parity tests (#16069) * minor fix: indent spaces in comment-out (#16076) * ci: print existing candidates (#16077) * [App] Fix bug where previously deleted apps cannot be re-run from the CLI (#16082) * Better check for programmatic lightningignore (#16080) Co-authored-by: Jirka Borovec <[email protected]> * [App] Removing single quote (#16079) * [App] PoC: Add support for Request (#16047) * Have checkgroup pull the latest runs (#16033) * Update Multinode Warning (#16091) * [App] Serve datatypes with better client code (#16018) * docs: add PT version (#16010) * docs: add PT version * stable Co-authored-by: Adrian Wälchli <[email protected]> Co-authored-by: Adrian Wälchli <[email protected]> * add 1.13.1 to adjust versions (#16099) * Remove redundant `find_unused_parameters=False` in Lite (#16026) * [App] Add display name property to the work (#16095) Co-authored-by: thomas <[email protected]> * Fix detection of whether app is running in cloud (#16045) * [App] Add work.delete (#16103) Co-authored-by: thomas <[email protected]> * [App] Improve the autoscaler UI (#16063) [App] Improve the autoscaler UI (#16063) * Re-enable Lite CLI on Windows + PyTorch 1.13 (#15645) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Justus Schock <[email protected]> * [App] Min replica=0 would break autoscaler component (#16092) * fixing the bug where num_replica=0 would fail * changelog * [App] Scale out/in interval for autoscaler (#16093) * Adding arguments for scale out/in interval * Tests * Set the default work start method to spawn on MacOS (#16089) * [App] Add status endpoint, enable `ready` (#16075) Co-authored-by: thomas chaton <[email protected]> * Clarify `work.stop()` limitation (#16073) * fix merge errors * Update torchvision requirement from <=0.14.0,>=0.11.1 to >=0.11.1,<0.15.0 in /requirements (#16108) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]> * CI: settle file names (#16098) * CI: settle file names * rename * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Fix test failing on master due to bad auto-merge (#16118) * fix merge error Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: Akihiro Nitta <[email protected]> Co-authored-by: thomas chaton <[email protected]> Co-authored-by: thomas <[email protected]> Co-authored-by: Yurij Mikhalevich <[email protected]> Co-authored-by: Ethan Harris <[email protected]> Co-authored-by: Sean Naren <[email protected]> Co-authored-by: Qiushi Pan <[email protected]> Co-authored-by: Jirka Borovec <[email protected]> Co-authored-by: Sherin Thomas <[email protected]> Co-authored-by: Justus Schock <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]>
* Remove the deprecated profiler imports (#16059) * Revert "Load app before setting LIGHTNING_DISPATCHED" (#16064) Revert "Load app before setting LIGHTNING_DISPATCHED (#16057)" This reverts commit 8d3339a. * [App] Hot fix: Resolve detection of python debugger (#16068) Co-authored-by: thomas <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> * Load the app before setting `LIGHTNING_DISPATCHED` (#16071) * fix(cloud): detect and ignore venv (#16056) Co-authored-by: Ethan Harris <[email protected]> * Add function to remove checkpoint to allow override for extended classes (#16067) * Drop FairScale sharded parity tests (#16069) * minor fix: indent spaces in comment-out (#16076) * ci: print existing candidates (#16077) * [App] Fix bug where previously deleted apps cannot be re-run from the CLI (#16082) * Better check for programmatic lightningignore (#16080) Co-authored-by: Jirka Borovec <[email protected]> * [App] Removing single quote (#16079) * [App] PoC: Add support for Request (#16047) * Have checkgroup pull the latest runs (#16033) * Update Multinode Warning (#16091) * [App] Serve datatypes with better client code (#16018) * docs: add PT version (#16010) * docs: add PT version * stable Co-authored-by: Adrian Wälchli <[email protected]> Co-authored-by: Adrian Wälchli <[email protected]> * add 1.13.1 to adjust versions (#16099) * Remove redundant `find_unused_parameters=False` in Lite (#16026) * [App] Add display name property to the work (#16095) Co-authored-by: thomas <[email protected]> * Fix detection of whether app is running in cloud (#16045) * [App] Add work.delete (#16103) Co-authored-by: thomas <[email protected]> * [App] Improve the autoscaler UI (#16063) [App] Improve the autoscaler UI (#16063) * Re-enable Lite CLI on Windows + PyTorch 1.13 (#15645) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Justus Schock <[email protected]> * [App] Min replica=0 would break autoscaler component (#16092) * fixing the bug where num_replica=0 would fail * changelog * [App] Scale out/in interval for autoscaler (#16093) * Adding arguments for scale out/in interval * Tests * Set the default work start method to spawn on MacOS (#16089) * [App] Add status endpoint, enable `ready` (#16075) Co-authored-by: thomas chaton <[email protected]> * Clarify `work.stop()` limitation (#16073) * fix merge errors * Update torchvision requirement from <=0.14.0,>=0.11.1 to >=0.11.1,<0.15.0 in /requirements (#16108) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]> * CI: settle file names (#16098) * CI: settle file names * rename * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Fix test failing on master due to bad auto-merge (#16118) * fix merge error Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: Akihiro Nitta <[email protected]> Co-authored-by: thomas chaton <[email protected]> Co-authored-by: thomas <[email protected]> Co-authored-by: Yurij Mikhalevich <[email protected]> Co-authored-by: Ethan Harris <[email protected]> Co-authored-by: Sean Naren <[email protected]> Co-authored-by: Qiushi Pan <[email protected]> Co-authored-by: Jirka Borovec <[email protected]> Co-authored-by: Sherin Thomas <[email protected]> Co-authored-by: Justus Schock <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]>
What does this PR do?
This PR speeds up
lightning run app app.py --cloud
by ignoring venv when uploading the app to the cloud.This PR makes
lightning run app app.py --cloud
create a.lightningignore
file if one doesn't exist:If the app detects that it's running inside of the
virtual env
dir it will create the following.lightningignore
:Does your PR introduce any breaking changes? If yes, please list them.
No.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃
cc @Borda