Skip to content

Python 3/urllib: imports, timeout - #9820

Merged
michaelDCurran merged 5 commits into
nvaccess:threshold_py3_stagingfrom
josephsl:py3_urllib
Jun 26, 2019
Merged

Python 3/urllib: imports, timeout#9820
michaelDCurran merged 5 commits into
nvaccess:threshold_py3_stagingfrom
josephsl:py3_urllib

Conversation

@josephsl

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #9818

Summary of the issue:

Update check functionality does not work due to incomplete imports.

Description of how this pull request fixes the issue:

Edited update check module:

  1. Import urllib.parse and urllib.request with an explanatory comment.
  2. urllib.urlopen -> urllib.request.urlopen
  3. urlib.urlencode -> urllib.parse.urlencode
  4. For timeout, urlopen function is now used to specify timeout, thus moved the code and comments for Updating without administrative privileges fails with some antivirus apps running in background #2352 around.
  5. Removed fp.soc line for specifying timeout, as urlopen now takes care of it.

Testing performed:

Tested with a custom build of Python 3 staging branch (with update version type specified) to simulate update check and via Python 3 interepreter.

Known issues with pull request:

More to be uncovered as update check module is converted.

Change log entry:

None

josephsl added 5 commits June 26, 2019 09:24
…lib. Re nvaccess#9818.

If one imports urllib in Python 3, only the bare bone package will be imported - no parsing, no errors, no opener, etc. Thus import both urllib.request and urllib.parse 9although just importing urllib.request will then load other parts of this package, imported both in update check module for sake of completeness).
…. Re nvaccess#9818.

In Python 3, timeout is specified as part of urllib.request.urlopen function. Therefore move the code around and the accompanying comment for nvaccess#2352.
@josephsl
josephsl requested a review from michaelDCurran June 26, 2019 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants