Chore/logging improvements real (#49)

Author: Juliya Smith

CHANGELOG.md

@@ -8,6 +8,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 The intended audience of this file is for py42 consumers -- as such, changes that don't affect
 how a consumer would use the library (e.g. adding unit tests, updating documentation, etc) are not captured here.
 
+## Unreleased
+
+### Added
+
+- Success messages for `profile delete` and `profile update`.
+- Additional information in the error log file:
+    - The full command path for the command that errored.
+    - User-facing error messages you see during adhoc sessions.
+
 ### 0.5.3 - 2020-05-04
 
 ### Fixed

setup.py

@@ -21,7 +21,7 @@
     package_dir={"": "src"},
     python_requires=">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, <4",
     install_requires=[
-        "c42eventextractor==0.2.7",
+        "c42eventextractor==0.2.9",
         "keyring==18.0.1",
         "keyrings.alt==3.2.0",
         "py42>=1.1.1",

src/code42cli/bulk.py

@@ -4,7 +4,7 @@
 
 from code42cli.compat import open, str
 from code42cli.worker import Worker
-from code42cli.errors import print_errors_occurred
+from code42cli.logger import get_main_cli_logger
 from code42cli.args import SDK_ARG_NAME, PROFILE_ARG_NAME
 
 
@@ -21,7 +21,7 @@ def generate_template(handler, path=None):
     ]
 
     if len(args) <= 1:
-        print(
+        get_main_cli_logger().print_info(
             u"A blank file was generated because there are no csv headers needed for this command. "
             u"Simply enter one {} per line.".format(args[0])
         )
@@ -103,9 +103,12 @@ def _process_flat_file_row(self, row):
     def _print_result(self):
         stats = self.__worker.stats
         successes = stats.total - stats.total_errors
-        print(u"{} processed successfully out of {}.".format(successes, stats.total))
+        logger = get_main_cli_logger()
+        logger.print_and_log_info(
+            u"{} processed successfully out of {}.".format(successes, stats.total)
+        )
         if stats.total_errors:
-            print_errors_occurred()
+            logger.print_errors_occurred_message()
 
 
 class CSVReader(object):

src/code42cli/cmds/detectionlists/__init__.py

@@ -1,6 +1,7 @@
+from code42cli.compat import str
 from code42cli.cmds.detectionlists.commands import DetectionListCommandFactory
 from code42cli.bulk import generate_template, run_bulk_process, CSVReader, FlatFileReader
-from code42cli.util import print_error
+from code42cli.logger import get_main_cli_logger
 from code42cli.cmds.detectionlists.enums import (
     BulkCommandType,
     DetectionLists,
@@ -148,7 +149,6 @@ def _remove_employee(self, sdk, profile, *args, **kwargs):
         self.handlers.remove_employee(sdk, profile, *args, **kwargs)
 
 
-
 def load_username_description(argument_collection):
     """Loads the arg descriptions for the `username` CLI parameter."""
     username = argument_collection.arg_configs[DetectionListUserKeys.USERNAME]
@@ -184,7 +184,7 @@ def get_user_id(sdk, username):
     users = sdk.users.get_by_username(username)[u"users"]
     if not users:
         ex = UserDoesNotExistError(username)
-        print_error(str(ex))
+        get_main_cli_logger().print_and_log_error(str(ex))
         raise ex
     return users[0][u"userUid"]
 

src/code42cli/cmds/profile.py

@@ -1,12 +1,13 @@
-from __future__ import print_function
-
 from getpass import getpass
 
 import code42cli.profile as cliprofile
+from code42cli.compat import str
+from code42cli.profile import print_and_log_no_existing_profile
 from code42cli.args import PROFILE_HELP, PROFILE_ARG_NAME
 from code42cli.commands import Command
 from code42cli.sdk_client import validate_connection
-from code42cli.util import does_user_agree, print_error, print_no_existing_profile_message
+from code42cli.util import does_user_agree
+from code42cli.logger import get_main_cli_logger
 
 
 def load_subcommands():
@@ -79,25 +80,27 @@ def load_subcommands():
 def show_profile(name=None):
     """Prints the given profile to stdout."""
     c42profile = cliprofile.get_profile(name)
-    print(u"\n{0}:".format(c42profile.name))
-    print(u"\t* username = {}".format(c42profile.username))
-    print(u"\t* authority url = {}".format(c42profile.authority_url))
-    print(u"\t* ignore-ssl-errors = {}".format(c42profile.ignore_ssl_errors))
+    logger = get_main_cli_logger()
+    logger.print_info(u"\n{0}:".format(c42profile.name))
+    logger.print_info(u"\t* username = {}".format(c42profile.username))
+    logger.print_info(u"\t* authority url = {}".format(c42profile.authority_url))
+    logger.print_info(u"\t* ignore-ssl-errors = {}".format(c42profile.ignore_ssl_errors))
     if cliprofile.get_stored_password(c42profile.name) is not None:
-        print(u"\t* A password is set.")
-    print(u"")
+        logger.print_info(u"\t* A password is set.")
+    logger.print_info(u"")
 
 
 def create_profile(profile, server, username, disable_ssl_errors=False):
     cliprofile.create_profile(profile, server, username, disable_ssl_errors)
     _prompt_for_allow_password_set(profile)
+    get_main_cli_logger().print_info(u"Successfully created profile '{}'.".format(profile))
 
 
 def update_profile(name=None, server=None, username=None, disable_ssl_errors=None):
     profile = cliprofile.get_profile(name)
     cliprofile.update_profile(profile.name, server, username, disable_ssl_errors)
     _prompt_for_allow_password_set(profile.name)
-    print(u"Profile '{}' has been updated.".format(profile.name))
+    get_main_cli_logger().print_info(u"Profile '{}' has been updated.".format(profile.name))
 
 
 def prompt_for_password_reset(name=None):
@@ -110,7 +113,8 @@ def prompt_for_password_reset(name=None):
 
 def _validate_connection(authority, username, password):
     if not validate_connection(authority, username, password):
-        print_error(
+        logger = get_main_cli_logger()
+        logger.print_and_log_error(
             u"Your credentials failed to validate, so your password was not stored."
             u"Check your network connection and the spelling of your username and server URL."
         )
@@ -120,11 +124,12 @@ def _validate_connection(authority, username, password):
 def list_profiles(*args):
     """Lists all profiles that exist for this OS user."""
     profiles = cliprofile.get_all_profiles()
+    logger = get_main_cli_logger()
     if not profiles:
-        print_no_existing_profile_message()
+        print_and_log_no_existing_profile()
         return
     for profile in profiles:
-        print(profile)
+        logger.print_info(str(profile))
 
 
 def use_profile(profile):
@@ -133,28 +138,31 @@ def use_profile(profile):
 
 
 def delete_profile(name):
+    logger = get_main_cli_logger()
     if cliprofile.is_default_profile(name):
-        print(u"\n{} is currently the default profile!".format(name))
+        logger.print_info(u"\n{} is currently the default profile!".format(name))
     if not does_user_agree(
-        u"\nDeleting this profile will also delete any stored passwords and checkpoints. Are you sure? (y/n): "
+        u"\nDeleting this profile will also delete any stored passwords and checkpoints. "
+        u"Are you sure? (y/n): "
     ):
         return
     cliprofile.delete_profile(name)
 
 
 def delete_all_profiles():
     existing_profiles = cliprofile.get_all_profiles()
+    logger = get_main_cli_logger()
     if existing_profiles:
-        print(u"\nAre you sure you want to delete the following profiles?")
+        logger.print_info(u"\nAre you sure you want to delete the following profiles?")
         for profile in existing_profiles:
-            print(u"\t{}".format(profile.name))
+            logger.print_info(u"\t{}".format(profile.name))
         if does_user_agree(
             u"\nThis will also delete any stored passwords and checkpoints. (y/n): "
         ):
             for profile in existing_profiles:
                 cliprofile.delete_profile(profile.name)
     else:
-        print(u"\nNo profiles exist. Nothing to delete.")
+        logger.print_info(u"\nNo profiles exist. Nothing to delete.")
 
 
 def _load_optional_profile_description(argument_collection):

src/code42cli/cmds/securitydata/extraction.py

@@ -1,8 +1,6 @@
-from __future__ import print_function
-
 import json
 
-from c42eventextractor import FileEventHandlers
+from c42eventextractor import ExtractionHandlers
 from c42eventextractor.extractors import FileEventExtractor
 from py42.sdk.queries.fileevents.filters import *
 
@@ -12,11 +10,10 @@
     IS_INCREMENTAL_KEY,
     SearchArguments,
 )
-from code42cli.logger import get_error_logger
 from code42cli.cmds.shared.cursor_store import FileEventCursorStore
 from code42cli.compat import str
-from code42cli.util import is_interactive, print_bold, print_error, print_to_stderr
 import code42cli.errors as errors
+from code42cli.logger import get_main_cli_logger
 
 
 _TOTAL_EVENTS = 0
@@ -62,18 +59,20 @@ def _determine_if_advanced_query(args):
         for key in given_args:
             val = given_args[key]
             if not _verify_compatibility_with_advanced_query(key, val):
-                print_error(u"You cannot use --advanced-query with additional search args.")
+                logger = get_main_cli_logger()
+                logger.print_and_log_error(
+                    u"You cannot use --advanced-query with additional search args."
+                )
                 exit(1)
         return True
     return False
 
 
 def _verify_begin_date_requirements(args, cursor_store):
     if _begin_date_is_required(args, cursor_store) and not args.begin:
-        print_error(u"'begin date' is required.")
-        print(u"")
-        print_bold(u"Try using  '-b' or '--begin'. Use `-h` for more info.")
-        print(u"")
+        logger = get_main_cli_logger()
+        logger.print_and_log_error(u"'begin date' is required.\n")
+        logger.print_bold(u"Try using  '-b' or '--begin'. Use `-h` for more info.\n")
         exit(1)
 
 
@@ -94,7 +93,8 @@ def _verify_exposure_types(exposure_types):
     options = list(ExposureTypeOptions())
     for exposure_type in exposure_types:
         if exposure_type not in options:
-            print_error(u"'{0}' is not a valid exposure type.".format(exposure_type))
+            logger = get_main_cli_logger()
+            logger.print_and_log_error(u"'{0}' is not a valid exposure type.".format(exposure_type))
             exit(1)
 
 
@@ -121,16 +121,16 @@ def _get_event_timestamp_filter(begin_date, end_date):
         end_date = end_date.strip() if end_date else None
         return date_helper.create_event_timestamp_filter(begin_date, end_date)
     except date_helper.DateArgumentException as ex:
-        print_error(str(ex))
+        get_main_cli_logger().print_and_log_error(str(ex))
         exit(1)
 
 
 def _create_event_handlers(output_logger, cursor_store):
-    handlers = FileEventHandlers()
-    error_logger = get_error_logger()
+    handlers = ExtractionHandlers()
 
     def handle_error(exception):
-        error_logger.error(exception)
+        logger = get_main_cli_logger()
+        logger.log_error(exception)
         errors.ERRORED = True
 
     handlers.handle_error = handle_error
@@ -172,9 +172,17 @@ def _verify_compatibility_with_advanced_query(key, val):
 def _handle_result():
     # Have to call this explicitly (instead of relying on invoker) because errors are caught in
     # `c42eventextractor`.
-    errors.print_errors_occurred_if_needed()
+    logger = get_main_cli_logger()
+    _print_errors_occurred_if_needed(logger)
     if not _TOTAL_EVENTS:
-        print_to_stderr(u"No results found\n")
+        logger.print_and_log_info(u"No results found.")
+
+
+def _print_errors_occurred_if_needed(logger):
+    """If interactive and errors occurred, it will print a message telling the user how to retrieve 
+    error logs."""
+    if errors.ERRORED:
+        logger.print_errors_occurred_message()
 
 
 def _try_append_exposure_types_filter(filters, include_non_exposure_events, exposure_types):
@@ -185,7 +193,9 @@ def _try_append_exposure_types_filter(filters, include_non_exposure_events, expo
 
 def _create_exposure_type_filter(include_non_exposure_events, exposure_types):
     if include_non_exposure_events and exposure_types:
-        print_error(u"Cannot use exposure types with `--include-non-exposure`.")
+        get_main_cli_logger().print_and_log_error(
+            u"Cannot use exposure types with `--include-non-exposure`."
+        )
         exit(1)
     if exposure_types:
         return ExposureType.is_in(exposure_types)

src/code42cli/cmds/securitydata/logger_factory.py

@@ -9,25 +9,23 @@
 from c42eventextractor.logging.handlers import NoPrioritySysLogHandlerWrapper
 
 from code42cli.cmds.securitydata.enums import OutputFormat
-from code42cli.util import get_url_parts, print_error
-from code42cli.logger import logger_has_handlers, logger_deps_lock, apply_logger_dependencies
+from code42cli.util import get_url_parts
+from code42cli.logger import (
+    logger_has_handlers,
+    logger_deps_lock,
+    add_handler_to_logger,
+    get_main_cli_logger,
+    get_logger_for_stdout as get_stdout_logger,
+)
 
 
 def get_logger_for_stdout(output_format):
     """Gets the stdout logger for the given format.
-
         Args:
             output_format: CEF, JSON, or RAW_JSON. Each type results in a different logger instance.
     """
-    logger = logging.getLogger(u"code42_stdout_{0}".format(output_format.lower()))
-    if logger_has_handlers(logger):
-        return logger
-
-    with logger_deps_lock:
-        if not logger_has_handlers(logger):
-            handler = logging.StreamHandler(sys.stdout)
-            return _init_logger(logger, handler, output_format)
-    return logger
+    formatter = _get_formatter(output_format)
+    return get_stdout_logger(output_format.lower(), formatter)
 
 
 def get_logger_for_file(filename, output_format):
@@ -69,7 +67,8 @@ def get_logger_for_server(hostname, protocol, output_format):
                     url_parts[0], port=port, protocol=protocol
                 ).handler
             except:
-                print_error(u"Unable to connect to {0}.".format(hostname))
+                logger = get_main_cli_logger()
+                logger.print_and_log_error(u"Unable to connect to {0}.".format(hostname))
                 exit(1)
             return _init_logger(logger, handler, output_format)
     return logger
@@ -78,7 +77,7 @@ def get_logger_for_server(hostname, protocol, output_format):
 def _init_logger(logger, handler, output_format):
     formatter = _get_formatter(output_format)
     logger.setLevel(logging.INFO)
-    return apply_logger_dependencies(logger, handler, formatter)
+    return add_handler_to_logger(logger, handler, formatter)
 
 
 def _get_formatter(output_format):