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
Accept logging level names (#5765) #5772
Conversation
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Seems like the parallel test has some fragility. I can't reproduce the same behavior when running virtual env on my machine. |
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
The functional tests was contaminated by env vars. |
self.assertEqual(logging.DEBUG, config.logging_level) | ||
|
||
def test_log_level_numbers_invalid(self): | ||
save(os.path.join(self.tmp_dir, CONAN_CONF), default_client_conf_log.format("level = wakawaka")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. Shouldn't we rise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from early condition, any error is treated as default logging level (CRITICAL). Should we change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition is here: https://github.com/conan-io/conan/blob/1.18.5/conans/client/conf/__init__.py#L511
@@ -506,7 +507,8 @@ def logging_level(self): | |||
if level is None: | |||
level = self.get_item("log.level") | |||
try: | |||
level = int(level) | |||
parsed_level = get_log_level_by_name(level) | |||
level = parsed_level if parsed_level is not None else int(level) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int("wakawaka") doesn't crash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Let's go ahead with this little change, I always have to think about the pairing between numbers and log-levels.
@jgsogo all yours. |
Please, @uilianries, consider these changes to keep the environment modification local to the test-case (uilianries#15) and I'll merge
Keep environment modification local to the testcase
@jgsogo Done, it has been merged! |
Perfect. Now move to the docs, we have the opportunity to improve them a little bit
Now any user can set Conan logging level using common names as debug, info, critical ...
Related Issue: #5765
Changelog: Feature: Accept logging level as logging names
Docs: conan-io/docs#1419
develop
branch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.