From 9b074cf8fbf3e1d431a44b0e59f19feb5987e05a Mon Sep 17 00:00:00 2001 From: Samira El Aabidi Date: Mon, 6 Jan 2020 11:27:05 +0200 Subject: [PATCH 1/3] Upgrade logger to be more customizable --- setup.py | 5 ++-- singer/__init__.py | 11 +------- singer/catalog.py | 8 +++--- singer/logger.py | 65 ++++++++++++++++++++++++++------------------- singer/messages.py | 6 ++--- singer/metrics.py | 12 ++++----- singer/transform.py | 18 ++++++------- 7 files changed, 63 insertions(+), 62 deletions(-) diff --git a/setup.py b/setup.py index 8df52bb..cd36b6d 100755 --- a/setup.py +++ b/setup.py @@ -1,10 +1,9 @@ #!/usr/bin/env python from setuptools import setup, find_packages -import subprocess setup(name="singer-python", - version='5.9.0', + version='5.10.0', description="Singer.io utility library", author="Stitch", classifiers=['Programming Language :: Python :: 3 :: Only'], @@ -15,7 +14,7 @@ 'simplejson==3.11.1', 'python-dateutil>=2.6.0', 'backoff==1.8.0', - 'ciso8601', + 'ciso8601', ], extras_require={ 'dev': [ diff --git a/singer/__init__.py b/singer/__init__.py index d3b2c87..2133be1 100644 --- a/singer/__init__.py +++ b/singer/__init__.py @@ -10,16 +10,7 @@ should_sync_field, ) -from singer.logger import ( - get_logger, - log_debug, - log_info, - log_warning, - log_error, - log_critical, - log_fatal, - log_exception, -) +from singer.logger import Logger from singer.metrics import ( Counter, diff --git a/singer/catalog.py b/singer/catalog.py index 1767ff1..a4013ba 100644 --- a/singer/catalog.py +++ b/singer/catalog.py @@ -4,16 +4,16 @@ from . import metadata as metadata_module from .bookmarks import get_currently_syncing -from .logger import get_logger +from .logger import Logger from .schema import Schema -LOGGER = get_logger() +LOGGER = Logger() def write_catalog(catalog): # If the catalog has no streams, log a warning if not catalog.streams: - LOGGER.warning("Catalog being written with no streams.") + LOGGER.log_warning("Catalog being written with no streams.") json.dump(catalog.to_dict(), sys.stdout, indent=2) @@ -150,7 +150,7 @@ def _shuffle_streams(self, state): def get_selected_streams(self, state): for stream in self._shuffle_streams(state): if not stream.is_selected(): - LOGGER.info('Skipping stream: %s', stream.tap_stream_id) + LOGGER.log_info('Skipping stream: %s', stream.tap_stream_id) continue yield stream diff --git a/singer/logger.py b/singer/logger.py index 2113f22..3d3ba86 100644 --- a/singer/logger.py +++ b/singer/logger.py @@ -1,45 +1,56 @@ import logging import logging.config import os +from typing import Optional -def get_logger(): - """Return a Logger instance appropriate for using in a Tap or a Target.""" - this_dir, _ = os.path.split(__file__) - path = os.path.join(this_dir, 'logging.conf') - # See - # https://docs.python.org/3.5/library/logging.config.html#logging.config.fileConfig - # for a discussion of why or why not to set disable_existing_loggers - # to False. The long and short of it is that if you don't set it to - # False it ruins external module's abilities to use the logging - # facility. - logging.config.fileConfig(path, disable_existing_loggers=False) - return logging.getLogger() +class Logger(object): + def __init__(self, config_file_path: Optional[str] = None, logger_name: Optional[str] = None): + """ + Creates a Logger instance appropriate for using in a Tap or a Target. + :param config_file_path: path to a custom logging file + :param logger_name: custom name for logger + """ -def log_debug(msg, *args, **kwargs): - get_logger().debug(msg, *args, **kwargs) + # fallback to singer's logging.conf if no file is given + if not config_file_path: + this_dir, _ = os.path.split(__file__) + config_file_path = os.path.join(this_dir, 'logging.conf') + # See + # https://docs.python.org/3.5/library/logging.config.html#logging.config.fileConfig + # for a discussion of why or why not to set disable_existing_loggers + # to False. The long and short of it is that if you don't set it to + # False it ruins external module's abilities to use the logging + # facility. + logging.config.fileConfig(config_file_path, disable_existing_loggers=False) -def log_info(msg, *args, **kwargs): - get_logger().info(msg, *args, **kwargs) + # use current file name as logger name + logger_name = __name__ if not logger_name else logger_name + self.__logger = logging.getLogger(logger_name) -def log_warning(msg, *args, **kwargs): - get_logger().warning(msg, *args, **kwargs) + def log_debug(self, msg, *args, **kwargs): + self.__logger.debug(msg, *args, **kwargs) + def log_info(self, msg, *args, **kwargs): + self.__logger.info(msg, *args, **kwargs) -def log_error(msg, *args, **kwargs): - get_logger().error(msg, *args, **kwargs) + def log_warning(self, msg, *args, **kwargs): + self.__logger.warning(msg, *args, **kwargs) + def log_error(self, msg, *args, **kwargs): + self.__logger.error(msg, *args, **kwargs) -def log_critical(msg, *args, **kwargs): - get_logger().critical(msg, *args, **kwargs) + def log_critical(self, msg, *args, **kwargs): + self.__logger.critical(msg, *args, **kwargs) + def log_fatal(self, msg, *args, **kwargs): + self.__logger.fatal(msg, *args, **kwargs) -def log_fatal(msg, *args, **kwargs): - get_logger().fatal(msg, *args, **kwargs) + def log_exception(self, msg, *args, **kwargs): + self.__logger.exception(msg, *args, **kwargs) - -def log_exception(msg, *args, **kwargs): - get_logger().exception(msg, *args, **kwargs) + def get_level(self): + return self.__logger.level diff --git a/singer/messages.py b/singer/messages.py index 3848801..cee6c55 100644 --- a/singer/messages.py +++ b/singer/messages.py @@ -5,8 +5,8 @@ import ciso8601 import singer.utils as u -from .logger import get_logger -LOGGER = get_logger() +from .logger import Logger +LOGGER = Logger() class Message(): '''Base class for messages.''' @@ -191,7 +191,7 @@ def parse_message(msg): try: time_extracted = ciso8601.parse_datetime(time_extracted) except: - LOGGER.warning("unable to parse time_extracted with ciso8601 library") + LOGGER.log_warning("unable to parse time_extracted with ciso8601 library") time_extracted = None diff --git a/singer/metrics.py b/singer/metrics.py index db32a41..b8b107d 100644 --- a/singer/metrics.py +++ b/singer/metrics.py @@ -44,10 +44,10 @@ import re import time from collections import namedtuple -from singer.logger import get_logger +from singer.logger import Logger DEFAULT_LOG_INTERVAL = 60 - +LOGGER = Logger() class Status: '''Constants for status codes''' @@ -84,7 +84,7 @@ def log(logger, point): 'value': point.value, 'tags': point.tags } - logger.info('METRIC: %s', json.dumps(result)) + logger.log_info('METRIC: %s', json.dumps(result)) class Counter(): @@ -118,7 +118,7 @@ def __init__(self, metric, tags=None, log_interval=DEFAULT_LOG_INTERVAL): self.value = 0 self.tags = tags if tags else {} self.log_interval = log_interval - self.logger = get_logger() + self.logger = Logger() self.last_log_time = time.time() def __enter__(self): @@ -173,7 +173,7 @@ class Timer(): # pylint: disable=too-few-public-methods def __init__(self, metric, tags): self.metric = metric self.tags = tags if tags else {} - self.logger = get_logger() + self.logger = Logger() self.start_time = None def __enter__(self): @@ -244,5 +244,5 @@ def parse(line): value=raw.get('value'), tags=raw.get('tags')) except Exception as exc: # pylint: disable=broad-except - get_logger().warning('Error parsing metric: %s', exc) + LOGGER.log_warning('Error parsing metric: %s', exc) return None diff --git a/singer/transform.py b/singer/transform.py index c165e38..b6de668 100644 --- a/singer/transform.py +++ b/singer/transform.py @@ -4,10 +4,10 @@ from jsonschema import RefResolver import singer.metadata -from singer.logger import get_logger +from singer.logger import Logger from singer.utils import (strftime, strptime_to_utc) -LOGGER = get_logger() +LOGGER = Logger() NO_INTEGER_DATETIME_PARSING = "no-integer-datetime-parsing" UNIX_SECONDS_INTEGER_DATETIME_PARSING = "unix-seconds-integer-datetime-parsing" @@ -24,7 +24,7 @@ def string_to_datetime(value): try: return strftime(strptime_to_utc(value)) except Exception as ex: - LOGGER.warning("%s, (%s)", ex, value) + LOGGER.log_warning("%s, (%s)", ex, value) return None @@ -89,20 +89,20 @@ def __init__(self, integer_datetime_fmt=NO_INTEGER_DATETIME_PARSING, pre_hook=No def log_warning(self): if self.filtered: - LOGGER.debug("Filtered %s paths during transforms " + LOGGER.log_debug("Filtered %s paths during transforms " "as they were unsupported or not selected:\n\t%s", len(self.filtered), "\n\t".join(sorted(self.filtered))) # Output list format to parse for reporting - LOGGER.debug("Filtered paths list: %s", + LOGGER.log_debug("Filtered paths list: %s", sorted(self.filtered)) if self.removed: - LOGGER.debug("Removed %s paths during transforms:\n\t%s", + LOGGER.log_debug("Removed %s paths during transforms:\n\t%s", len(self.removed), "\n\t".join(sorted(self.removed))) # Output list format to parse for reporting - LOGGER.debug("Removed paths list: %s", sorted(self.removed)) + LOGGER.log_debug("Removed paths list: %s", sorted(self.removed)) def __enter__(self): return self @@ -163,7 +163,7 @@ def transform_recur(self, data, schema, path): return success, transformed_data else: # pylint: disable=useless-else-on-loop # exhaused all types and didn't return, so we failed :-( - self.errors.append(Error(path, data, schema, logging_level=LOGGER.level)) + self.errors.append(Error(path, data, schema, logging_level=LOGGER.get_level())) return False, None def _transform_anyof(self, data, schema, path): @@ -174,7 +174,7 @@ def _transform_anyof(self, data, schema, path): return success, transformed_data else: # pylint: disable=useless-else-on-loop # exhaused all schemas and didn't return, so we failed :-( - self.errors.append(Error(path, data, schema, logging_level=LOGGER.level)) + self.errors.append(Error(path, data, schema, logging_level=LOGGER.get_level())) return False, None def _transform_object(self, data, schema, path, pattern_properties): From dfac4bbcbdff530bc6c53a3c063e2d7c26aed0c0 Mon Sep 17 00:00:00 2001 From: Samira-El <54845154+Samira-El@users.noreply.github.com> Date: Mon, 6 Jan 2020 11:28:49 +0200 Subject: [PATCH 2/3] keep version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index cd36b6d..8bc0cf2 100755 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ from setuptools import setup, find_packages setup(name="singer-python", - version='5.10.0', + version='5.9.0', description="Singer.io utility library", author="Stitch", classifiers=['Programming Language :: Python :: 3 :: Only'], From d5f84a1d2d1b1e0af0401024fb7a06ea44697597 Mon Sep 17 00:00:00 2001 From: Samira El Aabidi Date: Mon, 6 Jan 2020 12:06:33 +0200 Subject: [PATCH 3/3] pylinting --- singer/logger.py | 2 +- singer/transform.py | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/singer/logger.py b/singer/logger.py index 3d3ba86..4352d8e 100644 --- a/singer/logger.py +++ b/singer/logger.py @@ -4,7 +4,7 @@ from typing import Optional -class Logger(object): +class Logger: def __init__(self, config_file_path: Optional[str] = None, logger_name: Optional[str] = None): """ diff --git a/singer/transform.py b/singer/transform.py index b6de668..56cb5dd 100644 --- a/singer/transform.py +++ b/singer/transform.py @@ -48,6 +48,7 @@ def __init__(self, errors): super(SchemaMismatch, self).__init__(msg) + class SchemaKey: ref = "$ref" items = "items" @@ -55,6 +56,7 @@ class SchemaKey: pattern_properties = "patternProperties" any_of = 'anyOf' + class Error: def __init__(self, path, data, schema=None, logging_level=logging.INFO): self.path = path @@ -90,17 +92,17 @@ def __init__(self, integer_datetime_fmt=NO_INTEGER_DATETIME_PARSING, pre_hook=No def log_warning(self): if self.filtered: LOGGER.log_debug("Filtered %s paths during transforms " - "as they were unsupported or not selected:\n\t%s", - len(self.filtered), - "\n\t".join(sorted(self.filtered))) + "as they were unsupported or not selected:\n\t%s", + len(self.filtered), + "\n\t".join(sorted(self.filtered))) # Output list format to parse for reporting LOGGER.log_debug("Filtered paths list: %s", - sorted(self.filtered)) + sorted(self.filtered)) if self.removed: LOGGER.log_debug("Removed %s paths during transforms:\n\t%s", - len(self.removed), - "\n\t".join(sorted(self.removed))) + len(self.removed), + "\n\t".join(sorted(self.removed))) # Output list format to parse for reporting LOGGER.log_debug("Removed paths list: %s", sorted(self.removed)) @@ -161,7 +163,7 @@ def transform_recur(self, data, schema, path): success, transformed_data = self._transform(data, typ, schema, path) if success: return success, transformed_data - else: # pylint: disable=useless-else-on-loop + else: # pylint: disable=useless-else-on-loop # exhaused all types and didn't return, so we failed :-( self.errors.append(Error(path, data, schema, logging_level=LOGGER.get_level())) return False, None @@ -172,7 +174,7 @@ def _transform_anyof(self, data, schema, path): success, transformed_data = self.transform_recur(data, subschema, path) if success: return success, transformed_data - else: # pylint: disable=useless-else-on-loop + else: # pylint: disable=useless-else-on-loop # exhaused all schemas and didn't return, so we failed :-( self.errors.append(Error(path, data, schema, logging_level=LOGGER.get_level())) return False, None @@ -227,7 +229,7 @@ def _transform_array(self, data, schema, path): def _transform_datetime(self, value): if value is None or value == "": - return None # Short circuit in the case of null or empty string + return None # Short circuit in the case of null or empty string if self.integer_datetime_fmt not in VALID_DATETIME_FORMATS: raise Exception("Invalid integer datetime parsing option") @@ -332,10 +334,12 @@ def transform(data, schema, integer_datetime_fmt=NO_INTEGER_DATETIME_PARSING, transformer = Transformer(integer_datetime_fmt, pre_hook) return transformer.transform(data, schema, metadata=metadata) + def _transform_datetime(value, integer_datetime_fmt=NO_INTEGER_DATETIME_PARSING): transformer = Transformer(integer_datetime_fmt) return transformer._transform_datetime(value) + def resolve_schema_references(schema, refs=None): '''Resolves and replaces json-schema $refs with the appropriate dict. @@ -356,6 +360,7 @@ def resolve_schema_references(schema, refs=None): refs = refs or {} return _resolve_schema_references(schema, RefResolver("", schema, store=refs)) + def _resolve_schema_references(schema, resolver): if SchemaKey.ref in schema: reference_path = schema.pop(SchemaKey.ref, None)