Simplify updater logic for downloading and verifying target files #1202
Conversation
This internal method isn't used by any code other than tests. Signed-off-by: Joshua Lock <jlock@vmware.com>
The call stack and code for download_target() is more complex than required: * download_target() : builds target destination filepath, gets length and hashes * _get_target_file() : fixes filenames if consistent snapshots enabled, defines verification callback * _get_file() : iterates mirrors, tries to download files, verifies them Remove the verification callback and collapse the call stack by a single level to make the code easier to follow. Signed-off-by: Joshua Lock <jlock@vmware.com>
| try: | ||
| file_object = tuf.download.safe_download(file_mirror, file_length) | ||
|
|
||
| # Verify 'file_object' against the expected length and hashes. |
There was a problem hiding this comment.
Do you think it is possible to explain why there is a duplicated check of file length here after the one in safe_download?
There was a problem hiding this comment.
strictly speaking the checks that are done in functions that safe_download() calls are for the downloaded byte count, whereas _check_file_length() actually looks at the file object. I don't know how that could be of practical difference but...
There was a problem hiding this comment.
Good question, but I think better safe than sorry. I think redundant checks are fine as long as no real impact on performance.
Rather than read to the end of the file in order to determin its size, use the whence value of seek() to move the file object's position to the end of the file, then the tell() method of the file object to read the current position in bytes. Co-authored-by: Jussi Kukkonen <jkukkonen@vmware.com> Signed-off-by: Joshua Lock <jlock@vmware.com>
|
Trying close/re-open to handle the CI move |
Simplify the loop exit logic in _get_target_file() to simply return a verified file_object, once we have it, rather than breaking from the loop and then returning the file_object. This converts a use of a try/except/else to a try/except and is a little easier to read. Signed-off-by: Joshua Lock <jlock@vmware.com>
|
Why is it called |
with |
Description of the changes being introduced by the pull request:
The logic for downloading/verifying target files is fairly complex to follow and debug. This PR flattens several internal methods (including a callback) to make the logic simpler to follow.
Please verify and check that the pull request fulfills the following
requirements: