-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
Fix: variable become undefined when extension not exist or connection… #868
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -442,11 +442,12 @@ def _get_archives_base(self, name, target_packages): | |
try: | ||
extensions_xml_text = self._download_update_xml(extensions_xml_url, True) | ||
except ArchiveDownloadError: | ||
# In case _download_update_xml ignores the hash and tries to get the url. | ||
# In case _download_update_xml failed to get the url because of no extension. | ||
pass | ||
if extensions_xml_text: | ||
self.logger.info("Found extension {}".format(ext)) | ||
update_xmls.append(UpdateXmls(extensions_target_folder, extensions_xml_text)) | ||
else: | ||
if extensions_xml_text: | ||
self.logger.info("Found extension {}".format(ext)) | ||
update_xmls.append(UpdateXmls(extensions_target_folder, extensions_xml_text)) | ||
Comment on lines
+448
to
+450
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved this into the try block but putting them in the else block may be preferred. There are two places in metadata.py |
||
|
||
self._parse_update_xmls(update_xmls, target_packages) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The primary failure when the extension doesn't exist is ChecksumDownloadFailure, which is caught by download_update_xml. I had to modify the code to simulate getting through that and failing with an ArchiveDownloadError.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and I see in #867 the OP had set INSECURE_NOT_FOR_PRODUCTION_ignore_hash: True
So I think the original comment is more accurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
_download_update_xml
method failed to download sha256 checksum, it does not throw exception but setxml_hash = None
and callget_url
.The exception is thrown from
get_url
that try to downloadUpdates.xml
itself.So when got
ArchiveDownloadError
here, it means that it failed to downloadUpdate.xml
not hash.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming Settings.ignore_hash is False download_update_xml catches ChecksumDownloadFailure and RETURNS None. It doesn't call get_url (silent = True). This is the normal production behavior.
The OP set ignore_hash to True which behaves as you indicated.