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

Integration with notifiers #4

Closed
liiight opened this issue Dec 9, 2018 · 3 comments
Closed

Integration with notifiers #4

liiight opened this issue Dec 9, 2018 · 3 comments

Comments

@liiight
Copy link

@liiight liiight commented Dec 9, 2018

So I'm checking out this amazing library, which happens to be exactly what I was looking for, and to my great surprise, the last segment talks about integration with notifiers, which I'm its creator.

First off, I'm honored and flattened that you decided to include notifiers as a potential use case for your library. Thanks for that.

I'm not sure if you're aware of this, but I had the same idea in mind, so I created a custom logging handler for notifiers: https://notifiers.readthedocs.io/en/latest/Logger.html

Using it with logger.start could be even nicer with less boilerplate, which is kinda the point of this great library.

This is just a suggestion obviously, as your example is a perfectly valid use case.

Anyway, thanks again for mentioning notifiers and ofcourse, for this awesome library!

@Delgan
Copy link
Owner

@Delgan Delgan commented Dec 10, 2018

Hey @liiight! First, thanks for your kind words, I'm glad you like my library as much as I like notifiers. 😄

This is an great idea, I did not even think I could directly use the NotificationHandler provided by your library.

I made some tests, and I realized there was some kind of problem when using a logging.Handler as sink. The logged message is only formatted according to the logging.Formatter configured using setFormatter(). This is not very convenient. And this means that Loguru format parameter is actually ignored.
I have a very easy fix in mind, I will make some tests tomorrow.
Then I will update the README with your more elegant suggestion!

Notifications are so much useful, and you made it with a so simple API... Thanks to you!

@liiight
Copy link
Author

@liiight liiight commented Dec 10, 2018

That's great to hear. I'll be keeping a close eye on this project 😁

@Delgan
Copy link
Owner

@Delgan Delgan commented Dec 12, 2018

I improved the formatting of standard Handler and published a new version 0.2.2.

At the same time, I updated the Readme to mention the NotificationHandler from notifiers. Thanks again for having proposing the improvement! 👍

@Delgan Delgan closed this Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants