Skip to content

Python3: fix NVDA's logging infrastructure - #9712

Merged
michaelDCurran merged 4 commits into
threshold_py3_stagingfrom
py3_logHandler
Jun 12, 2019
Merged

Python3: fix NVDA's logging infrastructure#9712
michaelDCurran merged 4 commits into
threshold_py3_stagingfrom
py3_logHandler

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

Link to issue number:

None.

Summary of the issue:

In Python3: several issues cause NVDA's logging to fail:

  • NVDA makes use of logging._levelNames which no longer exists in Python3.
  • When NvDA calculates a code path when logging, it accesses a function's code object via its func_code property. In Python3 this is now __code__.
  • As strings are now unicode, and files that are opened in text mode deal in unicode, NVDA's logHandler.FileHandler no longer works.

Description of how this pull request fixes the issue:

  • No longer use logging._levelNames, rather use logging.getLevelName.
  • Changed logHandler.getCodePath to use __code__ instead of func_code.
  • logHandler.FileHandler now inherits from logging.Filehandler rather than the lower-level StreamHandler, and no longer converts to utf8 before writing. Rather the File object itself handles the utf8 encoding.

Testing performed:

Started NVDA under Python3. Info messages and tracebacks were successfully logged.

Known issues with pull request:

None.

Change log entry:

None.

Section: New features, Changes, Bug fixes

…reamHandler and assume unicode all the way until writing the file to disk with utf8.

logHandler: FunctionType.func_code is now FunctionType.__code__.

 # Please enter the commit message for your changes. Lines starting

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a few comments

Comment thread source/logHandler.py Outdated
Comment thread source/logHandler.py
Comment thread source/logHandler.py Outdated
Comment thread source/logHandler.py Outdated

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are still a few references to func_code in this file, could you check the rest of the code-base while you are at it? Could you also check for logging._levelNames in the rest of the code-base too?

Note, there are a few review comments from Leonard as well.

@michaelDCurran

Copy link
Copy Markdown
Member Author

I had previously grepped for any other uses of _levelNames and corrected them. There are none left.

@michaelDCurran

Copy link
Copy Markdown
Member Author

I have addressed review comments. Though didn't change the 'is' to an isinstance as this was existing code and I'm trying to avoid changing anything not necessary to change due to Python 3.

@michaelDCurran
michaelDCurran merged commit 0e06e45 into threshold_py3_staging Jun 12, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Jun 12, 2019
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