[App] Resolved root_folder not parsed properly #16454

tchaton · 2023-01-20T16:38:18Z

What does this PR do?

Resolve a race condition where the Drive would trigger a deep_diff to send a state update to api and for streamlit to send an outdated state.

Fixes #<issue_number>

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

Was this discussed/approved via a GitHub issue? (not for typos and docs)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together?
Did you make sure to update the documentation with your changes? (if necessary)
Did you write any new necessary tests? (not for typos and docs)
Did you verify new and existing tests pass locally with your changes?
Did you list all the breaking changes introduced by this pull request?
Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

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:

Is this pull request ready for review? (if not, please submit in draft mode)
Check that all items from Before submitting are resolved
Make sure the title is self-explanatory and the description concisely explains the PR
Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

cc @Borda

github-actions · 2023-01-20T16:38:53Z

⚡ Required checks status: All passing 🟢

Groups summary

🟢 lightning_app: Tests workflow

Check ID	Status
app-pytest (macOS-11, app, 3.8, latest)	success	✅
app-pytest (macOS-11, app, 3.8, oldest)	success	✅
app-pytest (macOS-11, lightning, 3.9, latest)	success	✅
app-pytest (ubuntu-20.04, app, 3.8, latest)	success	✅
app-pytest (ubuntu-20.04, app, 3.8, oldest)	success	✅
app-pytest (ubuntu-20.04, lightning, 3.9, latest)	success	✅
app-pytest (windows-2022, app, 3.8, latest)	success	✅
app-pytest (windows-2022, app, 3.8, oldest)	success	✅
app-pytest (windows-2022, lightning, 3.8, latest)	success	✅

These checks are required after the changes to src/lightning_app/core/app.py, src/lightning_app/storage/drive.py, src/lightning_app/utilities/proxies.py, tests/tests_app/core/test_lightning_app.py, tests/tests_app/core/test_lightning_flow.py, tests/tests_app/storage/test_drive.py.

🟢 lightning_app: Examples

Check ID	Status
app-examples (macOS-11, app, 3.9, latest)	success	✅
app-examples (macOS-11, app, 3.9, oldest)	success	✅
app-examples (macOS-11, lightning, 3.9, latest)	success	✅
app-examples (ubuntu-20.04, app, 3.9, latest)	success	✅
app-examples (ubuntu-20.04, app, 3.9, oldest)	success	✅
app-examples (ubuntu-20.04, lightning, 3.9, latest)	success	✅
app-examples (windows-2022, app, 3.9, latest)	success	✅
app-examples (windows-2022, app, 3.9, oldest)	success	✅
app-examples (windows-2022, lightning, 3.9, latest)	success	✅

These checks are required after the changes to src/lightning_app/core/app.py, src/lightning_app/storage/drive.py, src/lightning_app/utilities/proxies.py, tests/integrations_app/public/test_commands_and_api.py.

🟢 lightning_app: Azure

Check ID	Status
App.cloud-e2e	success	✅

These checks are required after the changes to src/lightning_app/core/app.py, src/lightning_app/storage/drive.py, src/lightning_app/utilities/proxies.py, tests/integrations_app/public/test_commands_and_api.py.

🟢 lightning_app: Docs

Check ID	Status
make-doctest (app)	success	✅
make-html (app)	success	✅

These checks are required after the changes to src/lightning_app/core/app.py, src/lightning_app/storage/drive.py, src/lightning_app/utilities/proxies.py.

🟢 mypy

Check ID	Status
mypy	success	✅

These checks are required after the changes to src/lightning_app/core/app.py, src/lightning_app/storage/drive.py, src/lightning_app/utilities/proxies.py.

🟢 install

Check ID	Status
install-pkg (ubuntu-22.04, app, 3.7)	success	✅
install-pkg (ubuntu-22.04, app, 3.10)	success	✅
install-pkg (ubuntu-22.04, fabric, 3.7)	success	✅
install-pkg (ubuntu-22.04, fabric, 3.10)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.7)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.10)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.7)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.10)	success	✅
install-pkg (ubuntu-22.04, notset, 3.7)	success	✅
install-pkg (ubuntu-22.04, notset, 3.10)	success	✅
install-pkg (macOS-12, app, 3.7)	success	✅
install-pkg (macOS-12, app, 3.10)	success	✅
install-pkg (macOS-12, fabric, 3.7)	success	✅
install-pkg (macOS-12, fabric, 3.10)	success	✅
install-pkg (macOS-12, pytorch, 3.7)	success	✅
install-pkg (macOS-12, pytorch, 3.10)	success	✅
install-pkg (macOS-12, lightning, 3.7)	success	✅
install-pkg (macOS-12, lightning, 3.10)	success	✅
install-pkg (macOS-12, notset, 3.7)	success	✅
install-pkg (macOS-12, notset, 3.10)	success	✅
install-pkg (windows-2022, app, 3.7)	success	✅
install-pkg (windows-2022, app, 3.10)	success	✅
install-pkg (windows-2022, fabric, 3.7)	success	✅
install-pkg (windows-2022, fabric, 3.10)	success	✅
install-pkg (windows-2022, pytorch, 3.7)	success	✅
install-pkg (windows-2022, pytorch, 3.10)	success	✅
install-pkg (windows-2022, lightning, 3.7)	success	✅
install-pkg (windows-2022, lightning, 3.10)	success	✅
install-pkg (windows-2022, notset, 3.7)	success	✅
install-pkg (windows-2022, notset, 3.10)	success	✅

These checks are required after the changes to src/lightning_app/core/app.py, src/lightning_app/storage/drive.py, src/lightning_app/utilities/proxies.py.

🟢 link-check

Check ID	Status
markdown-link-check	success	✅

These checks are required after the changes to src/lightning_app/CHANGELOG.md.

Thank you for your contribution! 💜

Note
This comment is automatically generated and updates for 60 minutes every 180 seconds. If you have any other questions, contact carmocca for help.

src/lightning_app/storage/drive.py

… into resolve_bug

src/lightning_app/core/app.py

lantiga

Good catch

Borda · 2023-01-24T17:46:39Z

tests/integrations_app/public/test_commands_and_api.py

-        assert resp.status_code == 200
-        assert resp.json() == "Hello World !"


Why were these assets removed? 😕
cc: @ethanwharris

Co-authored-by: thomas <[email protected]> (cherry picked from commit 404fc0c)

thomas added 3 commits January 20, 2023 15:46

update

1228799

add changelog

f8aedfc

update

d0b6188

tchaton requested review from lantiga, awaelchli, hhsecond and ethanwharris as code owners January 20, 2023 16:38

github-actions bot added the app (removed) Generic label for Lightning App package label Jan 20, 2023

thomas added 2 commits January 20, 2023 16:39

update

d76a130

update

d79419d

tchaton self-assigned this Jan 20, 2023

tchaton added the bug Something isn't working label Jan 20, 2023

tchaton added this to the future milestone Jan 20, 2023

tchaton commented Jan 20, 2023

View reviewed changes

src/lightning_app/storage/drive.py Show resolved Hide resolved

ethanwharris approved these changes Jan 20, 2023

View reviewed changes

thomas and others added 9 commits January 23, 2023 08:45

update

a77e91e

update

e2c4e1b

update

1ef4cc6

Merge branch 'master' into resolve_bug

8b6acc0

update

ec49d13

Merge branch 'resolve_bug' of https://github.com/Lightning-AI/lightning…

1435c3d

… into resolve_bug

update

3f51287

update

0475801

update

e72d7a2

hhsecond reviewed Jan 23, 2023

View reviewed changes

src/lightning_app/core/app.py Show resolved Hide resolved

hhsecond approved these changes Jan 23, 2023

View reviewed changes

mergify bot added the ready PRs ready to be merged label Jan 23, 2023

lantiga approved these changes Jan 23, 2023

View reviewed changes

awaelchli approved these changes Jan 23, 2023

View reviewed changes

thomas and others added 2 commits January 23, 2023 10:48

update

1f14bf9

Merge branch 'master' into resolve_bug

e487250

mergify bot added has conflicts and removed ready PRs ready to be merged labels Jan 23, 2023

Merge branch 'master' into resolve_bug

8b0706a

mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Jan 23, 2023

thomas added 2 commits January 23, 2023 14:37

update

f197389

update

9ab4019

tchaton merged commit 404fc0c into master Jan 23, 2023

tchaton deleted the resolve_bug branch January 23, 2023 15:03

Borda reviewed Jan 24, 2023

View reviewed changes

carmocca modified the milestones: future, v1.9.x Jan 25, 2023

Borda pushed a commit that referenced this pull request Feb 9, 2023

[App] Resolved root_folder not parsed properly (#16454)

a2788a5

Co-authored-by: thomas <[email protected]> (cherry picked from commit 404fc0c)

lantiga pushed a commit that referenced this pull request Feb 10, 2023

[App] Resolved root_folder not parsed properly (#16454)

0427412

Co-authored-by: thomas <[email protected]> (cherry picked from commit 404fc0c)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App] Resolved root_folder not parsed properly #16454

[App] Resolved root_folder not parsed properly #16454

tchaton commented Jan 20, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Jan 20, 2023 •

edited

Loading

lantiga left a comment

Borda Jan 24, 2023

		assert resp.status_code == 200
		assert resp.json() == "Hello World !"

[App] Resolved root_folder not parsed properly #16454

[App] Resolved root_folder not parsed properly #16454

Conversation

tchaton commented Jan 20, 2023 • edited by github-actions bot Loading

What does this PR do?

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

PR review

Did you have fun?

github-actions bot commented Jan 20, 2023 • edited Loading

⚡ Required checks status: All passing 🟢

Groups summary

lantiga left a comment

Choose a reason for hiding this comment

Borda Jan 24, 2023

Choose a reason for hiding this comment

tchaton commented Jan 20, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Jan 20, 2023 •

edited

Loading