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 logger #1328

Merged
merged 5 commits into from
Dec 10, 2019
Merged

Add logger #1328

merged 5 commits into from
Dec 10, 2019

Conversation

Jesus89
Copy link
Member

@Jesus89 Jesus89 commented Dec 10, 2019

Closes #422.

  • Replace prints by log.x
  • Add log_debug() and log_info() functions to set the log level

NOTE: this is the output of the logger, we can customize the message:

2019-12-10 09:49:27,922 - INFO - Success! Data uploaded correctly

@cmongut do you think we need to change this (remove time, log level, etc)?

@Jesus89 Jesus89 requested a review from alrocar December 10, 2019 08:55
This was referenced Dec 10, 2019
Copy link
Contributor

@alrocar alrocar left a comment

Choose a reason for hiding this comment

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

LGTM.

Just one question, how would a user of CARTOframes change the log level? Maybe we could document that in some place.

@Jesus89
Copy link
Member Author

Jesus89 commented Dec 10, 2019

Using the log_debug() and log_info() methods. I thought this is for internal use. Do you think we should expose and document it for all the users?

@alrocar
Copy link
Contributor

alrocar commented Dec 10, 2019

Sorry I meant a different thing.

Loggers allow to set the log level, so at runtime you decide if you want more verbose logs or not.

For example, now is set to INFO so I assume debug logs are not shown by default. As a user of CARTOframes how can I see the debug logs or only warnings and errors or in other words how can I set the log level?

@Jesus89
Copy link
Member Author

Jesus89 commented Dec 10, 2019

Yes, that's the goal of those functions :). We might improve the naming: set_log_level('debug'), set_log_level('info'), etc.

UPDATE: 4156780

@alrocar
Copy link
Contributor

alrocar commented Dec 10, 2019

sorry I misread them 🤦‍♂ the name change would help, thanks!

Copy link
Contributor

@simon-contreras-deel simon-contreras-deel left a comment

Choose a reason for hiding this comment

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

A detail. As a user, I would expect to have access to the log (since it is the Python logger) and set the level by myself, or even customizing how the log looks like.

For example, I could see all the CARTOframes logs doing:

from cartoframes.whatever.the.log.is import log
import logging

log.setLevel(logging.DEBUG)

@alrocar
Copy link
Contributor

alrocar commented Dec 10, 2019

Acceptance 🍏

@Jesus89 not sure if you want to include the requested changes by @oleurud in this PR or in a different one.

@simon-contreras-deel
Copy link
Contributor

Forget it. You could access to it by:

from cartoframes.core.logger import log 

I confused myself due to the function init_logger(), but it is called in the same module

@alrocar alrocar merged commit 2968666 into develop Dec 10, 2019
@alrocar alrocar deleted the 422-add-logger branch December 10, 2019 15:10
@alrocar
Copy link
Contributor

alrocar commented Dec 10, 2019

Cool, let's use this as much as possible :)

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.

Add better logging functionality with multiple levels
4 participants