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

Configure log level and log-color via commandline #14

Merged
merged 1 commit into from
Mar 15, 2019

Conversation

cfconrad
Copy link
Collaborator

Introduce new parameter options --loglevel and --log-color

@cfconrad cfconrad requested a review from jlausuch March 14, 2019 16:45
qatrfm/utils/logger.py Outdated Show resolved Hide resolved
@jlausuch
Copy link
Owner

Much better than exporting LOG_COLORS in environment. I just have a small question about aesthetics.. bold vs light.

Interestingly, Terraform CLI does actually the opposite, by default colours are enabled. Anyway, I prefer having them disabled by default because this tool will be mainly used in automated environments.

@@ -76,9 +76,15 @@ def find_tf_file(basedir):
@click.option('--no-clean', 'no_clean', is_flag=True,
help="Don't clean the environment when the tests finish. "
"This is useful for debug and troubleshooting.")
def cli(test, path, tfvar, snapshots, no_clean):
@click.option('--loglevel', 'loglevel', type=click.Choice([
'CRITICAL', 'ERROR', 'WARNING', 'INFO', 'DEBUG']),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Owner

@jlausuch jlausuch Mar 14, 2019

Choose a reason for hiding this comment

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

it's not reinventing the wheel. These codes are passed to the python logger later, which is exactly the link you pasted. See this line
This just allows the user to select which log level to display.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought something like this pallets/click#605 , but looks like it is not supported yet ...

@asmorodskyi
Copy link
Collaborator

LGTM

@jlausuch jlausuch merged commit 0a0c732 into jlausuch:master Mar 15, 2019
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.

3 participants