Skip to content

Commit

Permalink
Merge branch 'main' into standardize/805-reader_call_arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
srikanth-kumar authored Nov 5, 2024
2 parents ac539d3 + fa0b4f5 commit 37cc527
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
Refactoring Updates:
- description: |
*From GEOIPS#714: 7-26-24, Discuss how to share LOG attribute in the CLI*
Since we've started developing the GeoIPS CLI, we've known that there was a need to
have a shared logger object among all of the CLI command classes for debugging
purposes and to output basic information to the user if requested. We can now do
this using the '--log-level' / '-log' flag. Depending on the log level set, all
levels that match or are of higher 'order' than the log level provided will be
outputted. This functionality matches that used in 'run_procflow'. Logging is now
a supported utility throughout the CLI, and can be used in any CLI class via
self.LOG.<log_level>(log_statement). Note that self.LOG.<log_level> does not have to
match the provided '--log-level' ran in the command. This flag is strictly just the
LOG level filter. I.e. if '--log-level info' was provided, levels
['info', 'debug', 'interactive'] would be shown. The lower the level, the more
logging channels will output.
files:
modified:
- geoips/commandline/commandline_interface.py
- geoips/commandline/geoips_command.py
related-issue:
number: 714
repo_url: https://github.com/NRLMMD-GEOIPS/geoips
title: Share Logger Object to CLI Command Classes
41 changes: 29 additions & 12 deletions geoips/commandline/commandline_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@
from geoips.commandline.geoips_validate import GeoipsValidate


# Logging utitilies will be set up once PR 659, Apply Scope to CLI Arguments, has been
# merged. Until that point we'll set up no logging in the CLI as it will duplicate all
# log statements used in our procflows.


class GeoipsCLI(GeoipsCommand):
"""Top-Level Class for the GeoIPS Commandline Interface (CLI).
Expand Down Expand Up @@ -73,18 +68,40 @@ def __init__(self, instructions_dir=None, legacy=False):
# Otherwise use the default instructions which we know are correct
# (and if they're not, the appropriate error will be raised.)
self.cmd_instructions = None

# parse_known_args expects arguments in a specific order. So, currrently,
# 'geoips --log-level info <rest of command>' will work but
# 'geoips <rest of command> --log-level info' will not. The functionality below
# rearranges the log level arguments to match the working version. This way,
# users can add --log-level <log_level_name> anywhere in the command and it will
# work.
if set(sys.argv).intersection(set(["--log-level", "-log"])):
# One of the flags was found in the arguments provided
log_idx = max(
[
idx if arg in ["--log-level", "-log"] else -1
for idx, arg in enumerate(sys.argv)
]
)
# Make sure that the argument list is long enough for log level to be
# provided. It doesn't have to be correct, that validation will be done
# by argparse
if len(sys.argv) > log_idx + 1:
flag = sys.argv[log_idx]
log_level = sys.argv[log_idx + 1]
# Get the flag and log_level, remove them from the argument list, and
# insert them in working locations.
# I.e. geoips --log-level <log_level_name>
sys.argv.pop(log_idx + 1)
sys.argv.pop(log_idx)
sys.argv.insert(1, log_level)
sys.argv.insert(1, flag)

super().__init__(legacy=legacy)

def execute_command(self):
"""Execute the given command."""
self.GEOIPS_ARGS = self.parser.parse_args()
# NOTE: We should discuss how we want to share the selected LOG to child classes
# They can all use the functionality below, however that would be redundant and
# there is likely a better way to fix this. Unfortunately 'parse_known_args'
# didn't work and this is our best solution for the time being.

# self.LOG = getattr(LOG, self.GEOIPS_ARGS.log_level)
# self.LOG("LOG LEVEL = {self.GEOIPS_ARGS.log_level}")
if hasattr(self.GEOIPS_ARGS, "exe_command"):
# The command called is executable (child of GeoipsExecutableCommand)
# so execute that command now.
Expand Down
70 changes: 62 additions & 8 deletions geoips/commandline/geoips_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import yaml

from geoips.commandline.cmd_instructions import cmd_instructions, alias_mapping
from geoips.commandline.log_setup import setup_logging


class PluginPackages:
Expand Down Expand Up @@ -64,14 +65,15 @@ class ParentParsers:
shared correctly.
"""

geoips_parser = argparse.ArgumentParser(add_help=False)
geoips_parser = argparse.ArgumentParser()
geoips_parser.add_argument(
"--log_level",
"-log",
"--log-level",
type=str,
default="interactive",
choices=["debug", "error", "info", "interactive", "warning"],
help="The logging level to use for output via the CLI.",
# Specified in order of what will be shown. First is lowest level (10).
choices=["debug", "info", "warning", "interactive", "error", "critical"],
help="Log level to output when using the CLI.",
)

list_parser = argparse.ArgumentParser(add_help=False)
Expand Down Expand Up @@ -112,7 +114,7 @@ class GeoipsCommand(abc.ABC):
used for initializing command classes of a certain GeoIPS Command.
"""

def __init__(self, parent=None, legacy=False):
def __init__(self, LOG=None, parent=None, legacy=False):
"""Initialize GeoipsCommand with a subparser and default to the command func.
Do this for each GeoipsCLI.geoips_command_classes. This will instantiate
Expand All @@ -121,6 +123,11 @@ def __init__(self, parent=None, legacy=False):
Parameters
----------
LOG: optional - Logger Object
- Logging utility which can be used by any command class. Defaults to
LOG.interactive, however, can be changed to one of the values in
["debug", "error", "info", "interactive", "warning"] if
'--log_level/--log <log_level_name>' is specified at the command line.
parent: optional - GeoipsCommand Class
- The parent command class that possibly is initializing it's child.
Ex. GeoipsList would invoke this init function for each of its subcommand
Expand All @@ -133,6 +140,10 @@ def __init__(self, parent=None, legacy=False):
suppressing or displaying help information for '--procflow'.
"""
self.legacy = legacy
# This is the Logger Object, not the actual logger call function of 'log_level'.
# So, a command class would use the logger via:
# self.LOG.<log_level>(log_statement)
self.LOG = LOG
self.github_org_url = "https://github.com/NRLMMD-GEOIPS/"
self.parent = parent
self.alias_mapping = alias_mapping
Expand Down Expand Up @@ -200,9 +211,11 @@ def __init__(self, parent=None, legacy=False):
# Otherwise initialize a top-level parser for this command.
self.parser = argparse.ArgumentParser(
self.name,
conflict_handler="resolve",
parents=[ParentParsers.geoips_parser],
formatter_class=argparse.RawTextHelpFormatter,
)
self.LOG = self._get_cli_logger()
self.combined_name = self.name

self.add_subparsers()
Expand All @@ -211,6 +224,42 @@ def __init__(self, parent=None, legacy=False):
command_parser=self.parser,
)

def _get_cli_logger(self):
"""Set up and retrieve the logger object for use in the CLI.
If either flag ['--log-level', '--log'] was provided with a valid log level
after that flag, then set up the logger using the provided log level as the
filter for what will be logged.
Log Levels
----------
The log level filters what is logged when the CLI is ran. Filtering order shown
below. Log levels omit all levels that are below it:
- interactive
- debug
- info
- warning
- error
"""
# An independent parser is needed as this overrides the help messages of the CLI
# by providing ``add_help=False``, we keep the custom help messages for the CLI
# and by providing a parent class to the top level CLI parser, then it will be
# shown in the help messages as well.
independent_parser = argparse.ArgumentParser(add_help=False)
independent_parser.add_argument(
"-log",
"--log-level",
type=str,
default="interactive",
choices=["interactive", "debug", "info", "warning", "error"],
)
# Parse now, as we'll use logging among all of the child command classes
known_args, remaining_args = independent_parser.parse_known_args() # NOQA
log_level = known_args.log_level
# Set up logging based on the log level provided or defaulted to
LOG = setup_logging(logging_level=log_level.upper())
return LOG

@property
@abc.abstractmethod
def name(self):
Expand Down Expand Up @@ -243,7 +292,7 @@ def add_subparsers(self):
help=f"{self.name} instructions.",
)
for subcmd_cls in self.command_classes:
subcmd_cls(parent=self, legacy=self.legacy)
subcmd_cls(LOG=self.LOG, parent=self, legacy=self.legacy)

@property
def plugin_package_names(self):
Expand All @@ -263,7 +312,7 @@ class GeoipsExecutableCommand(GeoipsCommand):
can implement.
"""

def __init__(self, parent=None, legacy=False):
def __init__(self, LOG, parent=None, legacy=False):
"""Initialize GeoipsExecutableCommand.
This is a child of GeoipsCommand and will invoke the functionaly of
Expand All @@ -274,6 +323,11 @@ def __init__(self, parent=None, legacy=False):
Parameters
----------
LOG: Logger Object
- Logging utility which can be used by any command class. Defaults to
LOG.interactive, however, can be changed to one of the values in
["debug", "error", "info", "interactive", "warning"] if
'--log_level/--log <log_level_name>' is specified at the command line.
parent: optional - GeoipsCommand Class
- The parent command class that possibly is initializing it's child.
Ex. GeoipsList would invoke this init function for each of its subcommand
Expand All @@ -285,7 +339,7 @@ def __init__(self, parent=None, legacy=False):
the user called 'run_procflow' or 'data_fusion_procflow'. This is used for
suppressing or displaying help information for '--procflow'.
"""
super().__init__(parent=parent, legacy=legacy)
super().__init__(LOG=LOG, parent=parent, legacy=legacy)
# Since this class is exectuable (ie. not the cli, top-level list...),
# add available arguments for that command and set that function to
# the command's executable function (__call__) if that command is called.
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ test = [
"pytest-cov", # Reports on test coverage
"pixelmatch",
"xarray-datatree", # Currently only used in unit tests
"pytest", # Required for unit tests
"pytest-cov", # Reports on test coverage
"pixelmatch",
]
Expand Down

0 comments on commit 37cc527

Please sign in to comment.