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

improve for testings log_file_level = DEBUG #2277

Merged
merged 17 commits into from
May 22, 2024

Conversation

Preetam-Das26
Copy link
Contributor

Purpose of PR?:

Fixes #2221

Does this PR introduce a breaking change?

If the changes in this PR are manually verified, list down the scenarios covered::

Additional information for reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Does this PR results in some Documentation changes?
If yes, include the list of Documentation changes

Checklist:

  • Bug fix. Fixes #
  • New feature (Non-API breaking changes that adds functionality)
  • PR Title follows the convention of <type>: <subject>
  • Commit has unit tests

@Preetam-Das26
Copy link
Contributor Author

@ReimarBauer Please tell if the changes are correct.

@ReimarBauer
Copy link
Member

@joernu76 Do you want debug loggings for the wms?

@ReimarBauer
Copy link
Member

For my problem this change did not help, because https://github.com/Open-MSS/MSS/blob/develop/pytest.ini#L6
forces log_file_level = DEBUG not only in our library.

@Preetam-Das26
Copy link
Contributor Author

Does this work?

@joernu76
Copy link
Member

This looks fine!
However, the/a major problem is that the matplotlib library floods us with messages.

This is a humongous issue also with production servers.

Could you leave our stuff on DEBUG, but set the matplotlib logger to info? That would be very, veeery helpful.

@Preetam-Das26
Copy link
Contributor Author

Ok sure

@Preetam-Das26
Copy link
Contributor Author

Kindly check if the changes are correct. @ReimarBauer @joernu76

pytest.ini Outdated
@@ -3,7 +3,7 @@ log_cli = 1
log_cli_level = ERROR
log_cli_format = %(message)s
log_file = pytest.log
log_file_level = DEBUG
log_file_level = INFO
Copy link
Member

Choose a reason for hiding this comment

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

this should be kept DEBUG because pytest is only used for development.

When there are tests failing or the code has problems you usually want to show in pytest.log
and add debug messages.

@Preetam-Das26
Copy link
Contributor Author

Kindly check the changes @ReimarBauer

@@ -146,7 +146,7 @@ def verify_pw(username, password):
from mslib.utils.coordinate import get_projection_params

# Logging the Standard Output, which will be added to the Apache Log Files
logging.basicConfig(level=logging.DEBUG,
Copy link
Member

Choose a reason for hiding this comment

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

keep in the whole MSS code the existing DEBUG.

On the server site a user has to use their own plot class, data classes to extend the existing. Usually that did not work on first attempt. They need these debug messages.

@@ -55,6 +55,8 @@
OPENAIP_NOTICE = "Airspace data used comes from openAIP.\n" \
"Visit openAIP.net and contribute to better aviation data, free for everyone to use and share."
OURAIRPORTS_NOTICE = "Airports provided by OurAirports."
mpl_logger = logging.getLogger('matplotlib')
Copy link
Member

Choose a reason for hiding this comment

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

changing the mpl_logger is the needed solution.

but we need our own debug messages in all mslib modules

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

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

changing the mpl_logger is the needed solution.

but we need our own debug messages in all mslib modules

@ReimarBauer
Copy link
Member

Will that be continued?

@Preetam-Das26
Copy link
Contributor Author

Probably it cannot be done now.

@Preetam-Das26
Copy link
Contributor Author

Will this work @ReimarBauer ?

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

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

this is a solution for python 2 which enables a user to redefine the states, maybe we can create later a follow up on this idea if it can be practicable used in py3

https://github.com/moinwiki/moin-1.9/tree/master/wiki/config/logging

https://docs.python.org/3/library/logging.config.html#logging.config.fileConfig

the duplication of the lines may be improved

@@ -183,17 +183,17 @@ def sso_dir_path(local_file):
"stderr": {
"class": "logging.StreamHandler",
"stream": "ext:https://sys.stderr",
"level": "DEBUG",
"level": "INFO",
Copy link
Member

Choose a reason for hiding this comment

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

this was DEBUG

conftest.py Outdated
@@ -38,6 +40,11 @@
from mslib.mswms.demodata import DataFiles
import tests.constants as constants

matplotlib_logger = logging.getLogger('matplotlib')
matplotlib_logger.setLevel(logging.INFO)
mslib_logger = logging.getLogger('mslib')
Copy link
Member

Choose a reason for hiding this comment

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

this now changes any of our logging entries to debug. I think we should only change the 'matplotlib' logging

Copy link
Member

Choose a reason for hiding this comment

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

limit the code repetition by importing that, see above

@@ -94,6 +94,11 @@
"plot_title_size": "default",
"axes_label_size": "default"}

mpl_logger = logging.getLogger('matplotlib')
Copy link
Member

Choose a reason for hiding this comment

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

i would expect such blocks always after the import definition

and if it is always the same lines, can that become imported from an mslib.utils.loggerdef or other place?

@@ -0,0 +1,8 @@
# logging_config.py
Copy link
Member

Choose a reason for hiding this comment

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

please add the MSS header, see template


This file is part of MSS.

:copyright: Copyright 2017 Main Contributor
Copy link
Member

@ReimarBauer ReimarBauer May 21, 2024

Choose a reason for hiding this comment

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

What is your name @Preetam-Das26
and the year should be 2024, something like

:copyright: Copyright 2024 Preetam-Das26

This file is part of MSS.

:copyright: Copyright 2017 Main Contributor
:copyright: Copyright 2016-2023 by the MSS team, see AUTHORS.
Copy link
Member

Choose a reason for hiding this comment

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

fix the year too

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

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

Thx

@ReimarBauer ReimarBauer merged commit cabee4e into Open-MSS:develop May 22, 2024
10 checks passed
@Preetam-Das26 Preetam-Das26 deleted the log branch May 22, 2024 08:13
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.

improve for testings log_file_level = DEBUG
3 participants