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

Logging #12

Open
EMoonArchiTech opened this issue Sep 12, 2023 · 4 comments
Open

Logging #12

EMoonArchiTech opened this issue Sep 12, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@EMoonArchiTech
Copy link
Contributor

Hi All, I think it would be handy to implement some basic logging, just using the basic python logging module. I'm happy to do this myself and sprinkle some logging around the code if you guys aren't working on anything in the background.

@Yoshify
Copy link
Contributor

Yoshify commented Sep 12, 2023

Hello again,

I think we should err on the side of caution with logging as being a library, it should be up to the implementer to utilize their own logging solutions and do as they please with it. Some people don't like when libraries themselves are the noisy ones.

In saying that though - there definitely needs to be better error handling in place. More checks and balances would be good and it's on the TODO list. Logging/printing on a catch I think is okay - but we need to raise when things go wrong as well (so implementers can catch themselves).

@Yoshify Yoshify self-assigned this Sep 12, 2023
@Yoshify Yoshify added the enhancement New feature or request label Sep 12, 2023
@EMoonArchiTech
Copy link
Contributor Author

That's a good point regarding logging. I must admit I'm still in the dark about the finer points of developing for a module vs an application, as well as advanced usage of the logging library. Perhaps a configurable logging level in whatever way would be best-practice for logging inside of modules? It would at least give the option to turn info/debug logging on while using the module, or to do logging while developing for the module.

I have been thinking about error handling and how it could be approached. I think one easy improvement we could make is use the requests response.raise_for_status() function to have it raise an exception if it has a http error response code. I believe this function also gives more information than the existing endpoint _make_request() http error handling does (which iirc/afaik only raises errors with the error code.)

@Yoshify
Copy link
Contributor

Yoshify commented Sep 13, 2023

A flag for debug logging is a great idea and something worth looking into.

I'll be including some finer error handling with custom exceptions in a PR hopefully later tonight or tomorrow.

@Yoshify
Copy link
Contributor

Yoshify commented Sep 14, 2023

0.4.6 has been released with significantly better error handling and information around errors.
I've also introduced a new Config object that we can look into utilizing for passing in an optional logging object, or for toggling verbose logging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants