10000 Increase timeout for snapshot upload by ludeeus · Pull Request #43851 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Increase timeout for snapshot upload #43851

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

Merged
merged 1 commit into from
Dec 2, 2020
Merged

Conversation

ludeeus
Copy link
Member
@ludeeus ludeeus commented Dec 2, 2020

Breaking change

Proposed change

Increases the timeout between the client and HA for snapshot uploads from 10 to 300 seconds.
Fixes the 502 error you get on slower devices that uses > 10s for the upload.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant probot-home-assistant bot added core integration: hassio small-pr PRs with less than 30 lines. labels Dec 2, 2020
@ludeeus ludeeus marked this pull request as ready for review December 2, 2020 11:57
@@ -89,9 +90,10 @@ async def _command_proxy(
request._client_max_size = ( # pylint: disable=protected-access
MAX_UPLOAD_SIZE
)
client_timeout = 300
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not disable timeout?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but then it would never timeout 🤷
worst case you upload 1GB, with 5min it will still timeout if your connection is slower than 29Mb/s.
This should only be done in a local network, I think that is a reasonable max value?

But if you feel we should have None (disabled) I can update it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is this timeout only for the uploading, or also the processing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the upload, and only the first part of the upload (between the client (browser) and HA).
When that part is done, HA sends this to SU which will process it (save).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe 10min for slow SD?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing to SD is not a part of this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess 5 min is ok.

@pvizeli pvizeli added this to the 0.118.6 milestone Dec 2, 2020
@pvizeli pvizeli merged commit 15b5057 into dev Dec 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the supervisor-snapshot-upload-timeout branch December 2, 2020 13:34
@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2020
@frenck frenck modified the milestones: 0.118.6, 0.118.5 Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0