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

(enhancement): use python's logger instead of prints #11

Closed
aaronkaplan opened this issue Apr 26, 2023 · 3 comments · Fixed by #106
Closed

(enhancement): use python's logger instead of prints #11

aaronkaplan opened this issue Apr 26, 2023 · 3 comments · Fixed by #106
Assignees
Labels
enhancement New feature or request

Comments

@aaronkaplan
Copy link
Contributor

TBD if we want to use python's logger. That's a clearer way to separate log messages from print messages.. Related to #10 however, since the logger prefers late bindings and not f-strings.

@aaronkaplan aaronkaplan added the enhancement New feature or request label Apr 26, 2023
@aaronkaplan aaronkaplan self-assigned this Apr 26, 2023
@cvandeplas
Copy link
Contributor

And allow the user to stdout (by default), or save to a log using a cmd parameter.

dario-br added a commit that referenced this issue Oct 14, 2024
… present:

1. added logger at module level
2. replace print statement by correspoding logging statement: info, warning, error and debug (for commented prints)
@dario-br
Copy link
Contributor

@cvandeplas I did a first print replacement in the commit above. But now we need to change a couple of things, that I think they deserve discussion.

  1. All Parsers and Analysers must hold in the _result variable a dict and not also a list. And that dict must have a section/field named 'errors', as it already happens with some analysers. Why? Because we need to return an OK, NOK as a result of the operation. That should impact the methods parse and analyse of the Sysdiagnose class.
  2. Edit the main (CLI) function to (a) log those results and (b) configure the logging (for example, a file per case, per execution). All this subject to the cmd parameter.

dario-br added a commit that referenced this issue Oct 14, 2024
1. another demo parser??
2. get_result method when using cached results
dario-br added a commit that referenced this issue Oct 15, 2024
…onsole only as per cmd parameter (configured by default to WARNING).

File logging rules:
- JSONL format. Note: Still breaks if double quotes added to the message.
- File per execution (analyse, parse), per case.
@dario-br
Copy link
Contributor

I reverted a bit the mechanism suggested by @cvandeplas : it logs by default to jsonl (still breaks with double quotes) to any detail (INFO level), but it only logs to Console to the level defined by the cmd parameter.

Why? Because it behaves in the exact same way than parsers and analysers and because if there are long running parsing/analysing executions, you may want to see the details in a file on later stage (automation [wink]). However, unlike parsers and analysers that they only keep the last execution output, a new JSONL file is generated with every new execution.

dario-br added a commit that referenced this issue Oct 15, 2024
1. Use of proper JSON formatter
2. Use of extra and exc_info fields when logging. The usage of extra fields needs to be standardised
dario-br added a commit that referenced this issue Oct 15, 2024
dario-br added a commit that referenced this issue Oct 15, 2024
@dario-br dario-br linked a pull request Oct 15, 2024 that will close this issue
dario-br added a commit that referenced this issue Oct 16, 2024
dario-br added a commit that referenced this issue Oct 16, 2024
dario-br added a commit that referenced this issue Oct 16, 2024
dario-br added a commit that referenced this issue Oct 16, 2024
dario-br added a commit that referenced this issue Oct 16, 2024
dario-br added a commit that referenced this issue Oct 16, 2024
…parameter resides. That is, available to all modes
dario-br added a commit that referenced this issue Oct 17, 2024
dario-br added a commit that referenced this issue Oct 17, 2024
cvandeplas added a commit that referenced this issue Oct 21, 2024
Implements #11 - JSONL logging instead of printing to stdout
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

Successfully merging a pull request may close this issue.

3 participants