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

Temporary download names #747

Merged
merged 4 commits into from
Mar 17, 2024
Merged

Conversation

knoellle
Copy link
Contributor

Introduced Changes

#746 prevents uploading broken or truncated images. In my experience, the most common source of broken images or sdk files are interrupted downloads.
This PR should prevent most issues arising from interrupted downloads by using different filenames as download target and for checking whether the file has been downloaded already.
After the download finishes, the temporary file is moved into the correct place atomically, preventing half-finished files from ending up in the final location.

Fixes #

ToDo / Known Issues

Ideas for Next Iterations (Not This PR)

  • Maybe do the same atomic move after long interruptible process for sdk installation.
  • Resumable downloads? Probably not worth it unless we run into Iran 2018 internet conditions.

How to Test

Use with either pepsi gammaray or pepsi sdk install and abort during download. Then run the same command again and observe the result.
On main, gammaray would try to upload the truncated image file to the nao (careful, this can leave the robot in a broken state, improved by #746) and sdk installation would complain about broken file permissions, which isn't exactly helpful unless you already know what the issue is.
With this PR, the download should be simply reattempted upon running the command a second time.

@knoellle knoellle added tools:SDK Related to Yocto, the SDK et.al. tools:Tooling Related to pepsi et.al. labels Mar 16, 2024
@knoellle knoellle enabled auto-merge March 16, 2024 22:28
Copy link
Member

@schmidma schmidma left a comment

Choose a reason for hiding this comment

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

minor stuff

crates/repository/src/lib.rs Outdated Show resolved Hide resolved
crates/repository/src/lib.rs Outdated Show resolved Hide resolved
crates/repository/src/lib.rs Outdated Show resolved Hide resolved
crates/repository/src/lib.rs Outdated Show resolved Hide resolved
crates/repository/src/lib.rs Outdated Show resolved Hide resolved
crates/repository/src/lib.rs Outdated Show resolved Hide resolved
crates/repository/src/lib.rs Outdated Show resolved Hide resolved
crates/repository/src/lib.rs Outdated Show resolved Hide resolved
@knoellle knoellle added this pull request to the merge queue Mar 17, 2024
Merged via the queue into HULKs:main with commit b2eed73 Mar 17, 2024
20 checks passed
@knoellle knoellle deleted the temporary-download-names branch March 17, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools:SDK Related to Yocto, the SDK et.al. tools:Tooling Related to pepsi et.al.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants