PR: Use checksum to verify downloaded asset for updating Spyder (Update manager) #24131
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrclary
force-pushed
the
updater-download-verify
branch
from
1455137 to
e1a25ff
Compare
mrclary
force-pushed
the
updater-download-verify
branch
from
e1a25ff to
0fb3270
Compare
mrclary
force-pushed
the
updater-download-verify
branch
from
0fb3270 to
593e8f2
Compare
ccordoba12
reviewed
Apr 5, 2025
Member
ccordoba12
left a comment
ccordoba12
left a comment
There was a problem hiding this comment.
Just a few comments and suggestions for you @mrclary.
ccordoba12
changed the title
mrclary
commented
Apr 5, 2025
…set size. This can reduce url requests when checking for asset availability. This also obviates writing asset size to file. Verifying download now checks asset size against size info in Github release. Download directory is cleared immediately prior to download.
…eliminates the need to consider rate limits on Github url requests.
Use recommended Github request header. Cache get_asset_checksum, but only in unit tests. Create single checksum file and include digest for zip and pkg assets.
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
mrclary
force-pushed
the
updater-download-verify
branch
from
8c3b17b to
261d411
Compare
ccordoba12
reviewed
Apr 6, 2025
Member
There was a problem hiding this comment.
@mrclary, I tested this manually and have a few more suggestions/comments for you.
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
mrclary
commented
Apr 6, 2025
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
mrclary
force-pushed
the
updater-download-verify
branch
from
f3c36f8 to
e692d11
Compare
ccordoba12
approved these changes
Apr 7, 2025
Member
ccordoba12
left a comment
ccordoba12
left a comment
There was a problem hiding this comment.
Looks good to me now, thanks @mrclary!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the course of testing #24072, @ccordoba12 discovered the possibility that a previously downloaded update artifact may not match the current update artifact on Github. This is the result of using the completed download artifact size as the verification tool. This works fine to verify the download immediately upon download completion or if the Github artifact does not change. However, it will be more robust to use the checksum of the artifact to both verify the completed download and to verify an existing local artifact against the current Github artifact.
Spyder-checksums.txtasset. All 6.x release checksum assets are updated to reflect the new paradigm (unit tests will fail until this is completed).Spyder-checksums.txtasset is used to obtain the expected checksum for a downloaded asset. Fortunately, this does not need to be downloaded and read from file. A simple url request provides the contents of the file.get_github_releasesandget_asset_checksumare cached for unit tests. This removes the need to consider rate limit errors.Notes:
Spyder-checksums.txtfile can be used to verify one or more local assets from the command line as follows.updatemanagerunit tests rely on specific releases for testing and url requests for Github releases return a finite number of releases. This means that at some time in the near future, either the releases referenced in the tests will need to be updated or the number of releases returned by the url request will need to be increased. This issue existed prior to this PR.