Skip to content
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

[App] fix HTTPClient retry for flow/work queue #19837

Merged
merged 3 commits into from
May 1, 2024
Merged

Conversation

lantiga
Copy link
Collaborator

@lantiga lantiga commented May 1, 2024

What does this PR do?

Fixes issue with HTTPClient for the queue not retrying correctly in the app framework.

This PR changes the retry policy on the HTTPClient to:

  • include POST (even though it's not idempotent, but it's fine for our purpose)
  • include all relevant statuses (408, 429, 5xx)

📚 Documentation preview 📚: https://pytorch-lightning--19837.org.readthedocs.build/en/19837/

cc @Borda

@github-actions github-actions bot added the app (removed) Generic label for Lightning App package label May 1, 2024
Copy link
Contributor

github-actions bot commented May 1, 2024

⚡ Required checks status: All passing 🟢

Groups summary

🟢 lightning_app: Tests workflow
Check ID Status
app-pytest (macOS-11, lightning, 3.8, latest) success
app-pytest (macOS-11, lightning, 3.8, oldest) success
app-pytest (macOS-11, app, 3.9, latest) success
app-pytest (macOS-12, app, 3.11, latest) success
app-pytest (ubuntu-20.04, lightning, 3.8, latest) success
app-pytest (ubuntu-20.04, lightning, 3.8, oldest) success
app-pytest (ubuntu-20.04, app, 3.9, latest) success
app-pytest (ubuntu-22.04, app, 3.11, latest) success
app-pytest (windows-2022, lightning, 3.8, latest) success
app-pytest (windows-2022, lightning, 3.8, oldest) success
app-pytest (windows-2022, app, 3.8, latest) success
app-pytest (windows-2022, app, 3.11, latest) success

These checks are required after the changes to src/lightning/app/utilities/network.py, tests/tests_app/cli/test_cmd_install.py, tests/tests_app/utilities/test_network.py.

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

These checks are required after the changes to src/lightning/app/utilities/network.py.

🟢 lightning_app: Docs
Check ID Status
docs-make (app, doctest) success
docs-make (app, html) success

These checks are required after the changes to src/lightning/app/utilities/network.py.

🟢 mypy
Check ID Status
mypy success

These checks are required after the changes to src/lightning/app/utilities/network.py.

🟢 install
Check ID Status
install-pkg (ubuntu-22.04, app, 3.8) success
install-pkg (ubuntu-22.04, app, 3.11) success
install-pkg (ubuntu-22.04, fabric, 3.8) success
install-pkg (ubuntu-22.04, fabric, 3.11) success
install-pkg (ubuntu-22.04, pytorch, 3.8) success
install-pkg (ubuntu-22.04, pytorch, 3.11) success
install-pkg (ubuntu-22.04, lightning, 3.8) success
install-pkg (ubuntu-22.04, lightning, 3.11) success
install-pkg (ubuntu-22.04, notset, 3.8) success
install-pkg (ubuntu-22.04, notset, 3.11) success
install-pkg (macOS-12, app, 3.8) success
install-pkg (macOS-12, app, 3.11) success
install-pkg (macOS-12, fabric, 3.8) success
install-pkg (macOS-12, fabric, 3.11) success
install-pkg (macOS-12, pytorch, 3.8) success
install-pkg (macOS-12, pytorch, 3.11) success
install-pkg (macOS-12, lightning, 3.8) success
install-pkg (macOS-12, lightning, 3.11) success
install-pkg (macOS-12, notset, 3.8) success
install-pkg (macOS-12, notset, 3.11) success
install-pkg (windows-2022, app, 3.8) success
install-pkg (windows-2022, app, 3.11) success
install-pkg (windows-2022, fabric, 3.8) success
install-pkg (windows-2022, fabric, 3.11) success
install-pkg (windows-2022, pytorch, 3.8) success
install-pkg (windows-2022, pytorch, 3.11) success
install-pkg (windows-2022, lightning, 3.8) success
install-pkg (windows-2022, lightning, 3.11) success
install-pkg (windows-2022, notset, 3.8) success
install-pkg (windows-2022, notset, 3.11) success

These checks are required after the changes to src/lightning/app/utilities/network.py.


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.

@lantiga lantiga changed the title App: fix HTTPClient retry for flow/work queue [App] fix HTTPClient retry for flow/work queue May 1, 2024
@awaelchli awaelchli added the bug Something isn't working label May 1, 2024
@awaelchli awaelchli added this to the 2.2.x milestone May 1, 2024
@lantiga lantiga requested a review from rusenask May 1, 2024 21:05
@mergify mergify bot added the ready PRs ready to be merged label May 1, 2024
@lantiga lantiga merged commit d623708 into master May 1, 2024
86 checks passed
@lantiga lantiga deleted the app/fix-post-retry branch May 1, 2024 21:51
awaelchli added a commit that referenced this pull request May 1, 2024
Co-authored-by: Luca Antiga <luca@lightning.ai>
lantiga added a commit that referenced this pull request May 1, 2024
Co-authored-by: Luca Antiga <luca@lightning.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app (removed) Generic label for Lightning App package bug Something isn't working ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants