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

Define log level based on a config file #516

Open
PaulFarault opened this issue Nov 22, 2023 · 6 comments · May be fixed by #523
Open

Define log level based on a config file #516

PaulFarault opened this issue Nov 22, 2023 · 6 comments · May be fixed by #523
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@PaulFarault
Copy link
Contributor

For now, log level is set globally through the --log-level option on the CLI. The whole program and its dependency use this level.

It will be better to configure it using a file, to have more control over log level for each module (GitPython is very verbose in debug mode), and to benefit from it when using only the core lib.

@PaulFarault PaulFarault added enhancement New feature or request good first issue Good for newcomers labels Nov 22, 2023
@PaulFarault
Copy link
Contributor Author

PaulFarault commented Nov 22, 2023

Usage should be discussed, maybe add a -D/--debug option that takes the config file as an argument or pass it as an env var.

@mdrutel
Copy link
Contributor

mdrutel commented Nov 23, 2023

Good idea ^^
If it's a simple usage, I think we can add the used variable TDP_LOG_LEVEL in the environment file.
If it's a usage like log4j, I think we can use a config file as documented in your link.

@mdrutel
Copy link
Contributor

mdrutel commented Nov 23, 2023

Little questions :

  • if we use a config file, do we preserve the --log-level option ?
  • is the --debug option mandatory ?
  • In case --debug is optional, what is the default level or file config ?

@PaulFarault
Copy link
Contributor Author

This answer is only sharing my opinion, I'm open to suggestions.

if we use a config file, do we preserve the --log-level option ?

No.

is the --debug option mandatory ?

No, it should be optional.

In case --debug is optional, what is the default level or file config ?

Here are some thoughts:

  • The core lib should display WARNING, ERROR and CRITICAL by default. It can then be enhanced by using a configuration file.
  • I'm not sure that the CLI should display logs as we're using click.echo() to provide meaningful informations.
  • We could have 2 config files: one with "default" config (with warning, error, critical in particular) and one that is activated when the --debug option is set.
  • We could even have more configs and use -v, -vv, -vvv instead of --debug to switch between them but I'm not sure that we need that much granularity.

@rpignolet
Copy link
Collaborator

Be aware that CLI logs must be sent to stderr while the information requested by the user is sent to stdout. For example, a tdp browse displays a table, this must be displayed on stdout and all log lines must be in stderr.

This is necessary so that the tdp command can be used via the "pipe" (|) to do parsing for example.

@mdrutel mdrutel self-assigned this Nov 23, 2023
@mdrutel
Copy link
Contributor

mdrutel commented Nov 23, 2023

The content of the config file is described on this site
I think that if we implement this feature, it would be up to the user to define the behaviour he wants.

@mdrutel mdrutel linked a pull request Nov 24, 2023 that will close this issue
2 tasks
@mdrutel mdrutel linked a pull request Nov 24, 2023 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants