Skip to content

[feat] Add command line option for increasing the verbosity level of output #650

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jan 23, 2019
Merged

[feat] Add command line option for increasing the verbosity level of output #650

merged 8 commits into from
Jan 23, 2019

Conversation

@victorusu
Copy link
Contributor

@victorusu victorusu commented Jan 17, 2019

This PR adds the option to increase the verbosity level of the
PrettyPrinter. However, there is only one call to printer.debug and
there is on call to printer.verbose in the entire code. The former is
related to a module not being properly loaded, which is difficult to
reproduce, since we do have correct checks on ReFrame.

I am not sure if it is part of the issue to also add the printer.debug
and printer.verbose calls into the code.

@victorusu
Add option to increase output verbosity level
586f4da 
This PR adds the option to increase the verbosity level of the
PrettyPrinter. However, there is only one call to printer.debug and
there is on call to printer.verbose in the entire code. The former is
related to a module not being properly loaded, which is difficult to
reproduce, since we do have correct checks on ReFrame.

I am not sure if it is part of the issue to also add the printer.debug
and printer.verbose calls into the code.
@victorusu victorusu added this to the ReFrame sprint 2019w03 milestone Jan 17, 2019
@victorusu victorusu self-assigned this Jan 17, 2019
@victorusu victorusu requested review from omlins and vkarak January 17, 2019 09:39
@victorusu victorusu changed the title Add option to increase output verbosity level [feat] Add option to increase output verbosity level Jan 17, 2019
@vkarak vkarak mentioned this pull request Jan 17, 2019
Closed
@victorusu victorusu force-pushed the feature/verbositylevel branch from 92d1035 to 8022555 Compare January 18, 2019 13:25
@pep8speaks
Copy link

pep8speaks commented Jan 21, 2019 •
edited
Loading

Hello @victorusu, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated on January 23, 2019 at 23:07 Hours UTC

@codecov-io
Copy link

codecov-io commented Jan 21, 2019 •
edited
Loading

Codecov Report

Merging #650 into master will decrease coverage by <.01%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #650      +/-   ##
==========================================
- Coverage   91.73%   91.73%   -0.01%     
==========================================
  Files          76       76              
  Lines        9302     9326      +24     
==========================================
+ Hits         8533     8555      +22     
- Misses        769      771       +2
Impacted Files Coverage Δ
reframe/frontend/cli.py 80.88% <100%> (+0.19%) ⬆️
unittests/test_cli.py 96.41% <100%> (+0.1%) ⬆️
reframe/core/logging.py 84.22% <83.33%> (-0.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dcb29b...85a2b40. Read the comment docs.

vkarak
vkarak requested changes Jan 21, 2019
View reviewed changes
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

I think that by using the undocumented public fields of logging.Handler we will have a more straightforward implementation. We already use level when we hijack the setLevel method. I do also believe that the logging package is not extremely well documented in any case. We always had to dig into its code to understand what exactly goes on.

@vkarak vkarak changed the title [feat] Add option to increase output verbosity level [feat] Add command line option to increase output verbosity level Jan 22, 2019
@vkarak vkarak changed the title [feat] Add command line option to increase output verbosity level [feat] Add command line option to increase verbosity level of output Jan 22, 2019
vkarak
vkarak requested changes Jan 22, 2019
View reviewed changes
omlins
omlins approved these changes Jan 23, 2019
View reviewed changes
Copy link
Contributor

@omlins omlins left a comment

Choose a reason for hiding this comment

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

It looks good to me apart from the changes @vkarak requested.

vkarak
vkarak requested changes Jan 23, 2019
View reviewed changes
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Almost there. Could you also fix the PEP8 issues as well?

@vkarak vkarak self-assigned this Jan 23, 2019
Fine tune implementation
f225491 
- `inc_verbosity()` moved to the LoggerAdapter
- Fix a couple of bugs
- Fix coding style issues
@vkarak vkarak changed the title [feat] Add command line option to increase verbosity level of output [feat] Add command line option for increasing the verbosity level of output Jan 23, 2019
@vkarak vkarak merged commit f349a3e into reframe-hpc:master Jan 23, 2019
@vkarak vkarak mentioned this pull request Jan 29, 2019
Closed
@victorusu victorusu deleted the feature/verbositylevel branch October 6, 2020 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vkarak vkarak vkarak approved these changes

+1 more reviewer

@omlins omlins omlins approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vkarak vkarak

@victorusu victorusu

Labels

enhancement prio: normal

Projects

None yet

Milestone

ReFrame sprint 2019w03

Development

Successfully merging this pull request may close these issues.

5 participants

@victorusu @pep8speaks @codecov-io @vkarak @omlins