From febdbdbbf95f88b2a0975dd14638fc1c86435cbd Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Tue, 23 Feb 2021 20:55:29 +0000 Subject: [PATCH 1/8] Normalize the error messages --- tap_shopify/__init__.py | 31 ++++++++++++++++++++++--------- tap_shopify/exceptions.py | 3 +++ 2 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 tap_shopify/exceptions.py diff --git a/tap_shopify/__init__.py b/tap_shopify/__init__.py index f6cce69e..519b78a8 100644 --- a/tap_shopify/__init__.py +++ b/tap_shopify/__init__.py @@ -12,6 +12,7 @@ from singer import metadata from singer import Transformer from tap_shopify.context import Context +from tap_shopify.exceptions import ShopifyError import tap_shopify.streams # Load stream objects into Context REQUIRED_CONFIG_KEYS = ["shop", "api_key"] @@ -140,15 +141,27 @@ def sync(): Context.state['bookmarks']['currently_sync_stream'] = stream_id with Transformer() as transformer: - for rec in stream.sync(): - extraction_time = singer.utils.now() - record_schema = catalog_entry['schema'] - record_metadata = metadata.to_map(catalog_entry['metadata']) - rec = transformer.transform(rec, record_schema, record_metadata) - singer.write_record(stream_id, - rec, - time_extracted=extraction_time) - Context.counts[stream_id] += 1 + try: + for rec in stream.sync(): + extraction_time = singer.utils.now() + record_schema = catalog_entry['schema'] + record_metadata = metadata.to_map(catalog_entry['metadata']) + rec = transformer.transform(rec, record_schema, record_metadata) + singer.write_record(stream_id, + rec, + time_extracted=extraction_time) + Context.counts[stream_id] += 1 + except pyactiveresource.connection.ConnectionError as e: + msg = '' + try: + body_json = e.response.body.decode() + body = json.loads(body_json) + msg = body.get('errors') + finally: + raise ShopifyError(e, msg) from e + except pyactiveresource.connection.ServerError as e: + raise ShopifyError(e) from e + Context.state['bookmarks'].pop('currently_sync_stream') singer.write_state(Context.state) diff --git a/tap_shopify/exceptions.py b/tap_shopify/exceptions.py new file mode 100644 index 00000000..94fa62dd --- /dev/null +++ b/tap_shopify/exceptions.py @@ -0,0 +1,3 @@ +class ShopifyError(Exception): + def __init__(self, error, msg=''): + super().__init__('{}\n{}'.format(error.__class__.__name__, msg)) From 1448e9aba751372ff7ed0d92471c720c75735b83 Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Tue, 23 Feb 2021 21:01:19 +0000 Subject: [PATCH 2/8] Broaden the exception --- tap_shopify/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tap_shopify/__init__.py b/tap_shopify/__init__.py index 519b78a8..0155ab1c 100644 --- a/tap_shopify/__init__.py +++ b/tap_shopify/__init__.py @@ -159,7 +159,7 @@ def sync(): msg = body.get('errors') finally: raise ShopifyError(e, msg) from e - except pyactiveresource.connection.ServerError as e: + except Exception as e: raise ShopifyError(e) from e From 28ba615a39fad1516c853403de552cd2c9008e9f Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Tue, 23 Feb 2021 21:05:43 +0000 Subject: [PATCH 3/8] Put error message on a single line --- tap_shopify/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tap_shopify/exceptions.py b/tap_shopify/exceptions.py index 94fa62dd..d4ad8553 100644 --- a/tap_shopify/exceptions.py +++ b/tap_shopify/exceptions.py @@ -1,3 +1,3 @@ class ShopifyError(Exception): def __init__(self, error, msg=''): - super().__init__('{}\n{}'.format(error.__class__.__name__, msg)) + super().__init__('{}: {}'.format(error.__class__.__name__, msg)) From 6d2fc98fcff0bf093ca8ba9c2e0cd564d17584f5 Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Tue, 23 Feb 2021 21:11:49 +0000 Subject: [PATCH 4/8] Add helpful text in case of `ResourceNotFound` --- tap_shopify/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tap_shopify/__init__.py b/tap_shopify/__init__.py index 0155ab1c..3b82bf15 100644 --- a/tap_shopify/__init__.py +++ b/tap_shopify/__init__.py @@ -151,6 +151,8 @@ def sync(): rec, time_extracted=extraction_time) Context.counts[stream_id] += 1 + except pyactiveresource.connection.ResourceNotFound as e: + raise ShopifyError(e, 'Ensure shop is entered correctly') except pyactiveresource.connection.ConnectionError as e: msg = '' try: From 33b27ceb393bd6380cdc76f99e2c44201307b234 Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Wed, 24 Feb 2021 15:17:24 +0000 Subject: [PATCH 5/8] Change top level exception logger to log lines individually --- tap_shopify/__init__.py | 41 +++++++++++++++++++++------------------ tap_shopify/exceptions.py | 2 +- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/tap_shopify/__init__.py b/tap_shopify/__init__.py index 3b82bf15..e9e7855a 100644 --- a/tap_shopify/__init__.py +++ b/tap_shopify/__init__.py @@ -173,27 +173,30 @@ def sync(): LOGGER.info('%s: %d', stream_id, stream_count) LOGGER.info('----------------------') -@utils.handle_top_exception(LOGGER) def main(): - - # Parse command line arguments - args = utils.parse_args(REQUIRED_CONFIG_KEYS) - - # If discover flag was passed, run discovery mode and dump output to stdout - if args.discover: - catalog = discover() - print(json.dumps(catalog, indent=2)) - # Otherwise run in sync mode - else: - Context.tap_start = utils.now() - if args.catalog: - Context.catalog = args.catalog.to_dict() + try: + # Parse command line arguments + args = utils.parse_args(REQUIRED_CONFIG_KEYS) + + # If discover flag was passed, run discovery mode and dump output to stdout + if args.discover: + catalog = discover() + print(json.dumps(catalog, indent=2)) + # Otherwise run in sync mode else: - Context.catalog = discover() - - Context.config = args.config - Context.state = args.state - sync() + Context.tap_start = utils.now() + if args.catalog: + Context.catalog = args.catalog.to_dict() + else: + Context.catalog = discover() + + Context.config = args.config + Context.state = args.state + sync() + except Exception as e: + for line in str(e).splitlines(): + LOGGER.critical(line) + raise e if __name__ == "__main__": main() diff --git a/tap_shopify/exceptions.py b/tap_shopify/exceptions.py index d4ad8553..94fa62dd 100644 --- a/tap_shopify/exceptions.py +++ b/tap_shopify/exceptions.py @@ -1,3 +1,3 @@ class ShopifyError(Exception): def __init__(self, error, msg=''): - super().__init__('{}: {}'.format(error.__class__.__name__, msg)) + super().__init__('{}\n{}'.format(error.__class__.__name__, msg)) From fb59ad95a8d4a1d7dc92cddfedc3e4ae1532d80a Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Wed, 24 Feb 2021 15:22:22 +0000 Subject: [PATCH 6/8] Add helpful message in case of UnauthorizedAccess error --- tap_shopify/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tap_shopify/__init__.py b/tap_shopify/__init__.py index e9e7855a..7838fee5 100644 --- a/tap_shopify/__init__.py +++ b/tap_shopify/__init__.py @@ -153,6 +153,8 @@ def sync(): Context.counts[stream_id] += 1 except pyactiveresource.connection.ResourceNotFound as e: raise ShopifyError(e, 'Ensure shop is entered correctly') + except pyactiveresource.connection.UnauthorizedAccess as e: + raise ShopifyError(e, 'Invalid access token - Re-authorize the connection') except pyactiveresource.connection.ConnectionError as e: msg = '' try: From 271d3894966a8a391613d0e6655dc26fbd71550e Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Wed, 24 Feb 2021 15:45:28 +0000 Subject: [PATCH 7/8] Raise exceptions from causal exception --- Makefile | 2 +- tap_shopify/__init__.py | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 21529fb9..1bd8b48f 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ .DEFAULT_GOAL := test test: - pylint tap_shopify -d missing-docstring + pylint tap_shopify -d missing-docstring,too-many-branches nosetests tests/unittests diff --git a/tap_shopify/__init__.py b/tap_shopify/__init__.py index 7838fee5..eeb42c90 100644 --- a/tap_shopify/__init__.py +++ b/tap_shopify/__init__.py @@ -151,20 +151,21 @@ def sync(): rec, time_extracted=extraction_time) Context.counts[stream_id] += 1 - except pyactiveresource.connection.ResourceNotFound as e: - raise ShopifyError(e, 'Ensure shop is entered correctly') - except pyactiveresource.connection.UnauthorizedAccess as e: - raise ShopifyError(e, 'Invalid access token - Re-authorize the connection') - except pyactiveresource.connection.ConnectionError as e: + except pyactiveresource.connection.ResourceNotFound as exc: + raise ShopifyError(exc, 'Ensure shop is entered correctly') from exc + except pyactiveresource.connection.UnauthorizedAccess as exc: + raise ShopifyError(exc, 'Invalid access token - Re-authorize the connection') \ + from exc + except pyactiveresource.connection.ConnectionError as exc: msg = '' try: - body_json = e.response.body.decode() + body_json = exc.response.body.decode() body = json.loads(body_json) msg = body.get('errors') finally: - raise ShopifyError(e, msg) from e - except Exception as e: - raise ShopifyError(e) from e + raise ShopifyError(exc, msg) from exc + except Exception as exc: + raise ShopifyError(exc) from exc Context.state['bookmarks'].pop('currently_sync_stream') @@ -195,10 +196,10 @@ def main(): Context.config = args.config Context.state = args.state sync() - except Exception as e: - for line in str(e).splitlines(): + except Exception as exc: + for line in str(exc).splitlines(): LOGGER.critical(line) - raise e + raise exc if __name__ == "__main__": main() From d01c7047b5bd795490dfd8b6cf2b6753254efc15 Mon Sep 17 00:00:00 2001 From: Collin Simon Date: Thu, 25 Feb 2021 20:20:34 +0000 Subject: [PATCH 8/8] Bump singer-python to 5.11.0 and use `handle_top_exception` again --- setup.py | 2 +- tap_shopify/__init__.py | 40 ++++++++++++++++++---------------------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/setup.py b/setup.py index 955cf4bb..46e71b8b 100755 --- a/setup.py +++ b/setup.py @@ -11,7 +11,7 @@ py_modules=["tap_shopify"], install_requires=[ "ShopifyAPI==8.0.1", - "singer-python==5.9.1", + "singer-python==5.11.0", ], extras_require={ 'dev': [ diff --git a/tap_shopify/__init__.py b/tap_shopify/__init__.py index eeb42c90..34790862 100644 --- a/tap_shopify/__init__.py +++ b/tap_shopify/__init__.py @@ -176,30 +176,26 @@ def sync(): LOGGER.info('%s: %d', stream_id, stream_count) LOGGER.info('----------------------') +@utils.handle_top_exception(LOGGER) def main(): - try: - # Parse command line arguments - args = utils.parse_args(REQUIRED_CONFIG_KEYS) - - # If discover flag was passed, run discovery mode and dump output to stdout - if args.discover: - catalog = discover() - print(json.dumps(catalog, indent=2)) - # Otherwise run in sync mode + # Parse command line arguments + args = utils.parse_args(REQUIRED_CONFIG_KEYS) + + # If discover flag was passed, run discovery mode and dump output to stdout + if args.discover: + catalog = discover() + print(json.dumps(catalog, indent=2)) + # Otherwise run in sync mode + else: + Context.tap_start = utils.now() + if args.catalog: + Context.catalog = args.catalog.to_dict() else: - Context.tap_start = utils.now() - if args.catalog: - Context.catalog = args.catalog.to_dict() - else: - Context.catalog = discover() - - Context.config = args.config - Context.state = args.state - sync() - except Exception as exc: - for line in str(exc).splitlines(): - LOGGER.critical(line) - raise exc + Context.catalog = discover() + + Context.config = args.config + Context.state = args.state + sync() if __name__ == "__main__": main()