Fixing logging in mswms #2594
Fixing logging in mswms #2594
Conversation
| @@ -161,7 +160,7 @@ def plot_vsection(self, data, lats, lons, valid_time, init_time, | |||
| data[dataitem] = convert_to(data[dataitem], origunit, dataunit) | |||
| self.data_units[dataitem] = dataunit | |||
| else: | |||
| logging.debug("Please add units to plot variables") | |||
| mpl_logger.debug("Please add units to plot variables") | |||
There was a problem hiding this comment.
this likly will handle our needed output identical to the matplotlib logger which we want to supress in the output, or?
There was a problem hiding this comment.
look also on the tests results
There was a problem hiding this comment.
When you use just logging.debug() it uses a different logger (which is the root logger ) and not the matplotlib logger that we have defined. I am ensuring that we use our mpl_logger. This was a separate problem that I noticed.
There was a problem hiding this comment.
And about the original issue, I think we should be using common singleton logger. Hence, the statements
from mslib.utils import LOG
There was a problem hiding this comment.
And about the original issue, I think we should be using common singleton logger. Hence, the statements
from mslib.utils import LOG
There was a problem hiding this comment.
look also on the tests results
I did not get what you mean by this
There was a problem hiding this comment.
look also on the tests results
I did not get what you mean by this
There was a problem hiding this comment.
on bottom of your PR is a section with the CI Tests. Have a look on theire findings.
mslib/mswms/mswms.py
Outdated
|
|
||
| # keep the import after the version check. This creates all layers. | ||
| from mslib.mswms.wms import mswms_settings, server | ||
| from mslib.mswms.wms import mswms_settings, server, app as application |
There was a problem hiding this comment.
I have just moved the statement from top of the file to line 91. This is because we have logging statements in mslib.mswms.wms file and importing it will use logging without any configuration
There was a problem hiding this comment.
Sorry for the confusion, I had left the commented import app as application statement. Added a new commit.
|
Hi @anshulagrawal2902 see failing tests. |
|
Have a look on https://github.com/Open-MSS/MSS/actions/runs/12731829422/job/35510467794?pr=2594#step:4:74 For the first I guess maybe a feature of the IDE would avoid that on typing . |
|
I currently targeted only the |
|
there is a conflict shown, can you try to solve the merge conflict? |
Purpose of PR?:
Fixes #2500
Does this PR introduce a breaking change?
This does change the way we use logging, and also the way to select logging level from CLI
If the changes in this PR are manually verified, list down the scenarios covered::
Setting log level was tested on the mswms cli using
--logleveloptionAdditional information for reviewer? :
This PR is an attempt to have better logging accross the codebase
Does this PR results in some Documentation changes?
We would need to change the documention to add the
--logleveloption replacing--debugflagChecklist: