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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display log time #5

Merged
merged 1 commit into from Aug 13, 2022
Merged

Display log time #5

merged 1 commit into from Aug 13, 2022

Conversation

sjbitcode
Copy link
Contributor

@sjbitcode sjbitcode commented Jul 19, 2022

Summary

This PR adds an extra column to the rich Console output for displaying the time.

Changes

馃挕 In order to display a time, the logging handler needs a formatter that specifies a datefmt. This is needed so that the formatter can call formatTime, and we use that value to display!

  • Have DjangoRequestHandler.__init__ set a default formatter if none is passed in, else use passed in formatter
  • Have DjangoRequestHandler.emit call formatTime on its formatter and adds extra column to log output
  • Adds DjangoRequestHandler.__init__ tests 馃И

Testing

  1. Default formatter: Run the example project to see the default formatter at work! Time format will be HH:MM:SS
  2. Custom formatter: Add one of the pre-defined formatter to the django_rich_logging handler here. Time format will be whatever the formatter's datefmt setting is.
    • For example if you add "formatter": "verbose",, the time format will be YYYY-MM-DD HH:MM:SS

Notes, Questions, & Follow-up

  • Modify README to include note about optionally passing a formatter to the django_rich_logging handler
  • Should the tests check for the cell content?
    • current tests does minimal checking for the Method and text color
    • time checking would require something like freezegun?
  • Update gif?

Copy link
Contributor Author

@sjbitcode sjbitcode left a comment

Choose a reason for hiding this comment

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

Maybe its possible to not require a formatter and set a default on DjangoRequestHandler

@sjbitcode
Copy link
Contributor Author

I think this is ready for review now! I changed my approach twice 馃檭 Let me know if I need to change/add anything!

@adamghill adamghill merged commit 6f8e4fd into adamghill:main Aug 13, 2022
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.

None yet

2 participants