Skip to content
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

Deprecate MessageHandler. Rework all tools to use Python's logging #168

Open
amykyta3 opened this issue Apr 21, 2023 · 0 comments
Open

Deprecate MessageHandler. Rework all tools to use Python's logging #168

amykyta3 opened this issue Apr 21, 2023 · 0 comments
Assignees
Labels
feature New feature or request
Milestone

Comments

@amykyta3
Copy link
Member

amykyta3 commented Apr 21, 2023

Background

One long-standing issue about the compiler I have been meaning to address is the mechanism for reporting error & warning messages.
Rather than using Python's built-in logging library, I had for some reason implemented a custom message handling mechanism that is tightly bound to the mechanics of the compiler.

This is problematic for several reasons:

  • Poor control of message handling, filtering, and output
  • Difficult to use with compiler-adjacent tools like PeakRDL - one has to pass around a reference to the message handler class
  • Reinvents something that already exists - Python's logging facility

Changes

Migrate the systemrdl-compiler to emit messages using Python's logging facility.

Specifically, the following changes will be made:

  • Provide a new logging Formatter class that is specifically tailored for handling log messages that contain contextual RDL source referencing information. (basically provide a formatter that generates the exact same pretty error messages as before). To keep things easy to use, this formatter, along with a StreamHandler will be auto-loaded if no other handlers are found in the design.
  • Implement an equivalent mechanism for trapping and escalating errors during compilation & elaboration (escalate to RDLCompileError exception where appropriate)
  • Remove systemrdl.messages.MessagePrinter and systemrdl.messages.MessageHandler
  • Remove the optional message_printer argument to the RDLCompiler constructor
  • Remove references and usage of the MessageHandler at various points of the API (RDLCompiler.msg, RDLImporter.msg, and other commonly-used places)

Migration plan & compatibility

In nearly all cases, this will not affect anyone since I have not seen much reliance on overriding the default message printer, or other API usages. That said, since this represents a change to a small portion of the publicly-facing API, a proper deprecation cycle will be used in case these changes affect other users.
UPDATE: punting this to the v2.0 update of the compiler instead of hacking together a sloppy compatibility bridge.

If you think this change may affect you, be sure to subscribe to this issue page for updates.

@amykyta3 amykyta3 added the feature New feature or request label Apr 21, 2023
@amykyta3 amykyta3 self-assigned this Apr 21, 2023
@amykyta3 amykyta3 added this to the v2.0.0 milestone Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Development

No branches or pull requests

1 participant