From 006d7db87baf19c31c837460056df75dea37ddf5 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Sun, 27 Aug 2023 17:08:51 +1200 Subject: [PATCH 01/17] Handle replication key not found in stream schema --- singer_sdk/exceptions.py | 4 ++++ singer_sdk/streams/core.py | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/singer_sdk/exceptions.py b/singer_sdk/exceptions.py index 23325aa2a..351776291 100644 --- a/singer_sdk/exceptions.py +++ b/singer_sdk/exceptions.py @@ -17,6 +17,10 @@ class FatalAPIError(Exception): """Exception raised when a failed request should not be considered retriable.""" +class InvalidReplicationKeyException(Exception): + """Exception to raise if the replication key is not in the stream properties.""" + + class InvalidStreamSortException(Exception): """Exception to raise if sorting errors are found while syncing the records.""" diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 53e9533ff..8d68b3f9d 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -21,6 +21,7 @@ AbortedSyncPausedException, InvalidStreamSortException, MaxRecordsLimitException, + InvalidReplicationKeyException, ) from singer_sdk.helpers._batch import ( BaseBatchFileEncoding, @@ -215,6 +216,11 @@ def is_timestamp_replication_key(self) -> bool: if not self.replication_key: return False type_dict = self.schema.get("properties", {}).get(self.replication_key) + if type_dict is None: + msg = ( + f"{self.replication_key} is not in schema for stream name: {self.name}" + ) + raise InvalidReplicationKeyException(msg) return is_datetime_type(type_dict) def get_starting_replication_key_value( From 4ddf1c8d9faaf9ad5c3b00290264d63d96ba5ab4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 27 Aug 2023 05:29:39 +0000 Subject: [PATCH 02/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- singer_sdk/streams/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 8d68b3f9d..afc6d9d9d 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -19,9 +19,9 @@ from singer_sdk.exceptions import ( AbortedSyncFailedException, AbortedSyncPausedException, + InvalidReplicationKeyException, InvalidStreamSortException, MaxRecordsLimitException, - InvalidReplicationKeyException, ) from singer_sdk.helpers._batch import ( BaseBatchFileEncoding, From 89ddbb16f5efee1aa1e7b8a7532905a87bb3a09c Mon Sep 17 00:00:00 2001 From: mjsqu Date: Sun, 27 Aug 2023 17:34:56 +1200 Subject: [PATCH 03/17] Add missing Raises: to is_timestamp_replication_key docstring --- singer_sdk/streams/core.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index afc6d9d9d..490031529 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -212,6 +212,9 @@ def is_timestamp_replication_key(self) -> bool: Returns: True if the stream uses a timestamp-based replication key. + + Raises: + InvalidReplicationKeyException: If the schema does not contain the replication key. """ if not self.replication_key: return False From be1ef20fa35cfbc53326370eabd05c0ac5b4fce9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 27 Aug 2023 05:35:20 +0000 Subject: [PATCH 04/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- singer_sdk/streams/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 490031529..12b3f1e62 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -212,7 +212,7 @@ def is_timestamp_replication_key(self) -> bool: Returns: True if the stream uses a timestamp-based replication key. - + Raises: InvalidReplicationKeyException: If the schema does not contain the replication key. """ From 67c81220f26dbf1f96ca07571fc0527b1b1345b7 Mon Sep 17 00:00:00 2001 From: mjsqu Date: Sun, 27 Aug 2023 17:45:35 +1200 Subject: [PATCH 05/17] Split raise docstring over to new line for is_timestamp_replication_key docstring --- singer_sdk/streams/core.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 12b3f1e62..ef496ffcc 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -214,7 +214,8 @@ def is_timestamp_replication_key(self) -> bool: True if the stream uses a timestamp-based replication key. Raises: - InvalidReplicationKeyException: If the schema does not contain the replication key. + InvalidReplicationKeyException: If the schema does not contain the + replication key. """ if not self.replication_key: return False From 924a50d06370afc0971f0b26db8105d37d8279aa Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 27 Aug 2023 05:45:58 +0000 Subject: [PATCH 06/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- singer_sdk/streams/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index ef496ffcc..647adafd0 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -214,7 +214,7 @@ def is_timestamp_replication_key(self) -> bool: True if the stream uses a timestamp-based replication key. Raises: - InvalidReplicationKeyException: If the schema does not contain the + InvalidReplicationKeyException: If the schema does not contain the replication key. """ if not self.replication_key: From 516021a9f41ebf79fc90e1748b72d907ef56bd10 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Sun, 27 Aug 2023 17:51:27 +1200 Subject: [PATCH 07/17] Correct indentation for raises docstring --- singer_sdk/streams/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 647adafd0..5de42fd99 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -215,7 +215,7 @@ def is_timestamp_replication_key(self) -> bool: Raises: InvalidReplicationKeyException: If the schema does not contain the - replication key. + replication key. """ if not self.replication_key: return False From 7db509ecccccb2702dc57f3203729228b29a25c0 Mon Sep 17 00:00:00 2001 From: mjsqu Date: Mon, 28 Aug 2023 13:30:00 +1200 Subject: [PATCH 08/17] Add test_stream_invalid_replication_key --- tests/core/test_streams.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 34bbc7514..7c09e6860 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -275,6 +275,20 @@ def test_stream_starting_timestamp( assert get_starting_value(None) == expected_starting_value +def test_stream_invalid_replication_key(tap: SimpleTestTap): + """Validate an exception is raised if replication_key not in schema.""" + class InvalidReplicationKeyStream(SimpleTestStream): + replication_key = "INVALID" + + stream = InvalidReplicationKeyStream(tap) + + with pytest.raises( + InvalidReplicationKeyException, + match=f"{stream.replication_key} is not in schema for stream name: {stream.name}" + ): + stream.is_timestamp_replication_key + + @pytest.mark.parametrize( "path,content,result", [ From 90a8c4339e8c218d6f9463972dd368f10602ba63 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 28 Aug 2023 01:30:48 +0000 Subject: [PATCH 09/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/core/test_streams.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 7c09e6860..4f4215fa3 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -277,15 +277,16 @@ def test_stream_starting_timestamp( def test_stream_invalid_replication_key(tap: SimpleTestTap): """Validate an exception is raised if replication_key not in schema.""" + class InvalidReplicationKeyStream(SimpleTestStream): replication_key = "INVALID" stream = InvalidReplicationKeyStream(tap) - with pytest.raises( - InvalidReplicationKeyException, - match=f"{stream.replication_key} is not in schema for stream name: {stream.name}" - ): + with pytest.raises( + InvalidReplicationKeyException, + match=f"{stream.replication_key} is not in schema for stream name: {stream.name}", + ): stream.is_timestamp_replication_key From 9e9f77e4d636664ae289143497ec25984e040a44 Mon Sep 17 00:00:00 2001 From: mjsqu Date: Mon, 28 Aug 2023 13:34:15 +1200 Subject: [PATCH 10/17] Import InvalidReplicationKeyException name --- tests/core/test_streams.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 4f4215fa3..5d019a7f5 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -10,6 +10,9 @@ import requests from singer_sdk._singerlib import Catalog, MetadataMapping +from singer_sdk.exceptions import ( + InvalidReplicationKeyException, +) from singer_sdk.helpers._classproperty import classproperty from singer_sdk.helpers.jsonpath import _compile_jsonpath, extract_jsonpath from singer_sdk.pagination import first From 532b5af2f944ec2ec3a7c1eef5cfaaac68d5327d Mon Sep 17 00:00:00 2001 From: mjsqu Date: Mon, 28 Aug 2023 13:39:44 +1200 Subject: [PATCH 11/17] pre-commit check updates - line length and 'useless' expression --- tests/core/test_streams.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 5d019a7f5..5912d3ee9 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -288,9 +288,11 @@ class InvalidReplicationKeyStream(SimpleTestStream): with pytest.raises( InvalidReplicationKeyException, - match=f"{stream.replication_key} is not in schema for stream name: {stream.name}", + match=( + f"{stream.replication_key} is not in schema for stream name: {stream.name}" + ), ): - stream.is_timestamp_replication_key + exception_check = stream.is_timestamp_replication_key @pytest.mark.parametrize( From 8ccdc87628145e440be77874259d3f32c3ce9b35 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 28 Aug 2023 01:40:06 +0000 Subject: [PATCH 12/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/core/test_streams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 5912d3ee9..0be5f006d 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -292,7 +292,7 @@ class InvalidReplicationKeyStream(SimpleTestStream): f"{stream.replication_key} is not in schema for stream name: {stream.name}" ), ): - exception_check = stream.is_timestamp_replication_key + pass @pytest.mark.parametrize( From ff7de54f31e2a55897775565a8004778d1288463 Mon Sep 17 00:00:00 2001 From: mjsqu Date: Mon, 28 Aug 2023 13:45:21 +1200 Subject: [PATCH 13/17] Rename 'unused' variable with underscore prefix - variable is used to intentionally raise and exception --- tests/core/test_streams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 0be5f006d..1c63d5654 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -292,7 +292,7 @@ class InvalidReplicationKeyStream(SimpleTestStream): f"{stream.replication_key} is not in schema for stream name: {stream.name}" ), ): - pass + _check = stream.is_timestamp_replication_key @pytest.mark.parametrize( From 34906b5f9c63d089c56710066652ce529e6c974a Mon Sep 17 00:00:00 2001 From: mjsqu Date: Tue, 29 Aug 2023 06:41:19 +1200 Subject: [PATCH 14/17] Improve error message Co-authored-by: Edgar R. M. --- singer_sdk/streams/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 5de42fd99..2d6d842fa 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -222,7 +222,7 @@ def is_timestamp_replication_key(self) -> bool: type_dict = self.schema.get("properties", {}).get(self.replication_key) if type_dict is None: msg = ( - f"{self.replication_key} is not in schema for stream name: {self.name}" + f"Field '{self.replication_key}' is not in schema for stream '{self.name}'" ) raise InvalidReplicationKeyException(msg) return is_datetime_type(type_dict) From 4415bcc797ded79bc8c793da1f9ca7979e2efb41 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 28 Aug 2023 18:41:41 +0000 Subject: [PATCH 15/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- singer_sdk/streams/core.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 2d6d842fa..798387295 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -221,9 +221,7 @@ def is_timestamp_replication_key(self) -> bool: return False type_dict = self.schema.get("properties", {}).get(self.replication_key) if type_dict is None: - msg = ( - f"Field '{self.replication_key}' is not in schema for stream '{self.name}'" - ) + msg = f"Field '{self.replication_key}' is not in schema for stream '{self.name}'" raise InvalidReplicationKeyException(msg) return is_datetime_type(type_dict) From 8b1da60c12d0ab894d2226c627a7a71f10756787 Mon Sep 17 00:00:00 2001 From: mjsqu Date: Tue, 29 Aug 2023 06:47:49 +1200 Subject: [PATCH 16/17] Update exception message in invalid replication key test Co-authored-by: Edgar R. M. --- tests/core/test_streams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 1c63d5654..efa4066eb 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -289,7 +289,7 @@ class InvalidReplicationKeyStream(SimpleTestStream): with pytest.raises( InvalidReplicationKeyException, match=( - f"{stream.replication_key} is not in schema for stream name: {stream.name}" + f"Field '{stream.replication_key}' is not in schema for stream '{stream.name}'" ), ): _check = stream.is_timestamp_replication_key From 0ca1e27b489afe15680337a6f1992f5b71b10823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez=20Mondrag=C3=B3n?= Date: Mon, 28 Aug 2023 13:08:11 -0600 Subject: [PATCH 17/17] Make linter happy --- singer_sdk/streams/core.py | 2 +- tests/core/test_streams.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/singer_sdk/streams/core.py b/singer_sdk/streams/core.py index 798387295..4c3adb225 100644 --- a/singer_sdk/streams/core.py +++ b/singer_sdk/streams/core.py @@ -221,7 +221,7 @@ def is_timestamp_replication_key(self) -> bool: return False type_dict = self.schema.get("properties", {}).get(self.replication_key) if type_dict is None: - msg = f"Field '{self.replication_key}' is not in schema for stream '{self.name}'" + msg = f"Field '{self.replication_key}' is not in schema for stream '{self.name}'" # noqa: E501 raise InvalidReplicationKeyException(msg) return is_datetime_type(type_dict) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index efa4066eb..a3a451086 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -289,7 +289,8 @@ class InvalidReplicationKeyStream(SimpleTestStream): with pytest.raises( InvalidReplicationKeyException, match=( - f"Field '{stream.replication_key}' is not in schema for stream '{stream.name}'" + f"Field '{stream.replication_key}' is not in schema for stream " + f"'{stream.name}'" ), ): _check = stream.is_timestamp_replication_key