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

Share log attr between cli classes #759

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
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
69 changes: 61 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,14 @@ 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.",
choices=["interactive", "debug", "info", "warning", "error"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line should be reverted to the previous order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems interactive is set to level 35. Debug is 10, info is 20, warning is 30, error is 40, critical is 50. So the list should actually be in the order of [debug, info, warning, interactive, error, critical].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has since been addressed.

help="Log level to output when using the CLI.",
)

list_parser = argparse.ArgumentParser(add_help=False)
Expand Down Expand Up @@ -112,7 +113,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 +122,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 +139,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 +210,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 +223,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 +291,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 +311,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 +322,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 +338,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
Loading