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

Stale download cancellation #5025

Closed
wants to merge 8 commits into from

Conversation

twz123
Copy link
Member

@twz123 twz123 commented Sep 23, 2024

Description

Cancel downloads whenever there's no data flowing for a certain amount of time. Add a very long overall timeout for Autopilot downloads. This will ensure that the download will fail at some point, even if the remote server is artificially slow.

Add k0scontext.WithInactivityTimeout, which returns a context that times out after a specified period of inactivity. It provides a "keep alive" mechanism to reset the timeout based on recent activity, ensuring that the context will remain valid for as long as there is activity. This is used to implement the cancellation of stale downloads.

Move the private writerFunc implementation from the integration tests into the regular code base. This adapter is useful from time to time.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist:

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

This is a simple HTTP download function that can be used to download
stuff from the Internet. Special care is taken to detect and sanitize
server-suggested file names. Intended to replace the dependency on grab.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
It may very well be a local variable instead.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
This makes the download process use a temporary file and atomic file
replacements on success. Allows existing files to be re-downloaded,
which was not possible before. On the contrary, it will re-download files
even if they didn't change.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Returns a context that times out after a specified period of inactivity.
It provides a "keep alive" mechanism to reset the timeout based on
recent activity, ensuring that the context will remain valid for as long
as there is activity.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Moves the private implementation from the integration tests into the
regular code base. This adapter is useful from time to time.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Cancel downloads whenever there's no data flowing for a certain amount
of time. Use k0scontext.WithInactivityTimeout for that.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
This will ensure that the download will fail at some point, even if the
remote server is artificially slow.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Copy link
Contributor

This pull request has merge conflicts that need to be resolved.

@twz123
Copy link
Member Author

twz123 commented Sep 26, 2024

As we now have #5042 that can be backported, this PR can now be part of #5020 again.

@twz123 twz123 closed this Sep 26, 2024
@twz123 twz123 deleted the stale-download-cancellation branch September 26, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants