-
-
Notifications
You must be signed in to change notification settings - Fork 104
Open
Labels
Description
Summary
The logutil module contains components that are either broken, or overly broad in this project responsibilities, and have not been properly maintained or tested for a long time.
I propose to fix the NTDebugHandler class and deprecate the setup_logging function.
Problems
1. NTDebugHandler is broken due to Python 2/3 str vs bytes incompatibility
Lines 17 to 28 in 06879ab
| class NTDebugHandler(logging.Handler): | |
| def emit( | |
| self, | |
| record, | |
| writeA=_OutputDebugStringA, | |
| writeW=_OutputDebugStringW, | |
| ): | |
| text = self.format(record) | |
| if isinstance(text, str): | |
| writeA(text + "\n") | |
| else: | |
| writeW(text + "\n") |
- In Python 2,
strrepresented byte sequences, and Unicode strings were handled by a separateunicodetype.
In Python 3,strrepresents Unicode text, andbytesis used explicitly for byte sequences. - In Python 3,
logging.Formatter.format, which is invoked bylogging.Logger.format, always returns a Unicode string (str). - The
NTDebugHandler.emitmethod has a type check (isinstance(text, str)) and passing thisstrdirectly to_OutputDebugStringA(which expectsLPCSTR(bytes)) inevitably leads to errors. - So
_OutputDebugStringW(for wide character strings) is sufficient. The existing type branching is now a source of malfunction.
2. setup_logging has excessive responsibility
- The
setup_loggingis a justloggingutility, unrelated to COM or Windows. - Such
loggingconfiguration responsibilities should ideally rest with the application developer.
3. Lack of usage and redundancy
- These components are likely unused or used in very limited circumstances for a long time.
NTDebugHandleris used inserver/inprocserver.py, but as discussed in Are there any use cases forDllCanUnloadNowandDllGetClassObject? #671, the use case forinprocserverhas not been confirmed for years.
This suggests either that its breakage has gone unnoticed, or developers are using forks/custom patches.- The
setup_loggingfunction is not called anywhere withinserver/inprocserver.py, indicating its non-essential nature for the project and a weak justification for its existence.
Proposal
For the reasons outlined above, we propose to fix NTDebugHandler and deprecate the setup_logging function from this library.
This will simplify the codebase, resolve potential Python version incompatibility issues, and clarify the scope of this project responsibilities.
I plan to fix NTDebugHandler by the next release and deprecate setup_logging, with a view to removing it in a future version (likely 1.5.0).
Reactions are currently unavailable