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

Add a logger config file example and how to use it #2260

Closed
wants to merge 2 commits into from
Closed

Add a logger config file example and how to use it #2260

wants to merge 2 commits into from

Conversation

bijela-gora
Copy link

Summary

Hello,

Thank you for paying attention to this.

I've added an example how to use logger config file and how it can look like. The changes should resolve #491

Thank you in advance for a review.

Checklist

  • I've updated the documentation
  • I've checked how changes to the doc page looks like, by using the scripts/docs serve command
  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)

New section screenshot

image

@bijela-gora
Copy link
Author

@Kludex @tomchristie hi,

I'd appreciate it if you could review this PR. Thank you in advance.

Sorry, for mentioning you. Details and a screenshot in the first PR comment.

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

Looks really neat. ☺️

@bijela-gora
Copy link
Author

bijela-gora commented Mar 2, 2024

Thank you!

What else should be done to squash merge this? I don't have a merge button.

@Kludex Kludex added the hold Don't merge yet label Mar 2, 2024
@Kludex
Copy link
Member

Kludex commented Mar 2, 2024

I don't think we should be recommending any third-party package here.

I also think we need a whole section about logging.

@Kludex
Copy link
Member

Kludex commented Mar 2, 2024

@bijela-gora
Copy link
Author

bijela-gora commented Mar 2, 2024

@Kludex

Hello, thank you for the review.

I decided to take structured login as an example because it's widely adopted, it's useful and valuable. I fully understand and accept your desire not to recommend any third-party packages, even though I don't have the same experience in open source as you. For me it will be even more interesting to use Python native abilities to implement structured logging.

During preparation of the section about --log-config parameter, I felt tendency to write a lot more then is written, to show the whole example. But then I realized that it looks like a tutorial about logging, and not a short example. So, I stopped myself. This PR was created just to close the #491 issue.

If you'll allow me, I can only speak for myself, I was looking for exactly this kind of section. About structural logging. It is now a de-facto industry standard. I agree that there is still a lot to be said about logging. But I also believe that even such a small improvement will be useful.

So, before I go on, I want to ask you if you will accept my PR if I remove the third-party package recommendation?

By the way, I checked the source code of python-json-logger and it looks small and quite simple, means easily auditable.

@bijela-gora bijela-gora closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold Don't merge yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add example of logging config file
3 participants