From 108a87cb24500d5dd6d81abeb87e39f6c8699fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Wed, 29 Jun 2022 21:24:11 -0500 Subject: [PATCH 01/14] feat: Validate unsupported config options --- singer_sdk/exceptions.py | 21 +++++++ singer_sdk/plugin_base.py | 5 +- singer_sdk/tap_base.py | 24 +++++--- singer_sdk/typing.py | 13 +++-- tests/core/test_jsonschema_helpers.py | 84 +++++++++++++++++++++------ tests/core/test_streams.py | 55 +++++++++++++++--- 6 files changed, 163 insertions(+), 39 deletions(-) diff --git a/singer_sdk/exceptions.py b/singer_sdk/exceptions.py index eddc7af52..773a07253 100644 --- a/singer_sdk/exceptions.py +++ b/singer_sdk/exceptions.py @@ -1,10 +1,31 @@ """Defines a common set of exceptions which developers can raise and/or catch.""" + +from __future__ import annotations + import requests class ConfigValidationError(Exception): """Raised when a user's config settings fail validation.""" + def __init__( + self, + message: str, + *, + errors: list[str] | None = None, + warnings: list[str] | None = None, + ) -> None: + """Initialize a ConfigValidationError. + + Args: + message: A message describing the error. + errors: A list of errors which caused the validation error. + warnings: A list of warnings which caused the validation error. + """ + super().__init__(message) + self.errors = errors or [] + self.warnings = warnings or [] + class FatalAPIError(Exception): """Exception raised when a failed request should not be considered retriable.""" diff --git a/singer_sdk/plugin_base.py b/singer_sdk/plugin_base.py index 7ef4d4857..df93dcf7a 100644 --- a/singer_sdk/plugin_base.py +++ b/singer_sdk/plugin_base.py @@ -250,7 +250,7 @@ def _validate_config( f"JSONSchema was: {config_jsonschema}" ) if raise_errors: - raise ConfigValidationError(summary) + raise ConfigValidationError(summary, errors=errors) log_fn = self.logger.warning else: @@ -259,7 +259,8 @@ def _validate_config( summary += f"\n{warning}" if warnings_as_errors and raise_errors and warnings: raise ConfigValidationError( - f"One or more warnings ocurred during validation: {warnings}" + f"One or more warnings ocurred during validation: {warnings}", + warnings=warnings, ) log_fn(summary) return warnings, errors diff --git a/singer_sdk/tap_base.py b/singer_sdk/tap_base.py index 75f1b481e..fd31a2361 100644 --- a/singer_sdk/tap_base.py +++ b/singer_sdk/tap_base.py @@ -9,7 +9,7 @@ import click from singer_sdk.cli import common_options -from singer_sdk.exceptions import MaxRecordsLimitException +from singer_sdk.exceptions import ConfigValidationError, MaxRecordsLimitException from singer_sdk.helpers import _state from singer_sdk.helpers._classproperty import classproperty from singer_sdk.helpers._compat import final @@ -453,6 +453,7 @@ def cli( Raises: FileNotFoundError: If the config file does not exist. + Abort: If the configuration is not valid. """ if version: cls.print_version() @@ -486,13 +487,20 @@ def cli( config_files.append(Path(config_path)) - tap = cls( # type: ignore # Ignore 'type not callable' - config=config_files or None, - state=state, - catalog=catalog, - parse_env_config=parse_env_config, - validate_config=validate_config, - ) + try: + tap = cls( # type: ignore # Ignore 'type not callable' + config=config_files or None, + state=state, + catalog=catalog, + parse_env_config=parse_env_config, + validate_config=validate_config, + ) + except ConfigValidationError as exc: + for error in exc.errors: + click.secho(error, fg="red") + for warning in exc.warnings: + click.secho(warning, fg="warning") + raise click.Abort("Configuration is not valid.") if discover: tap.run_discovery() diff --git a/singer_sdk/typing.py b/singer_sdk/typing.py index 77417a620..8ec79dc96 100644 --- a/singer_sdk/typing.py +++ b/singer_sdk/typing.py @@ -403,14 +403,14 @@ class ObjectType(JSONTypeHelper): def __init__( self, *properties: Property, - additional_properties: W | type[W] | None = None, + additional_properties: W | type[W] | bool | None = None, ) -> None: """Initialize ObjectType from its list of properties. Args: properties: Zero or more attributes for this JSON object. additional_properties: A schema to match against unnamed properties in - this object. + this object or a boolean indicating if extra properties are allowed. """ self.wrapped: list[Property] = list(properties) self.additional_properties = additional_properties @@ -428,13 +428,16 @@ def type_dict(self) -> dict: # type: ignore # OK: @classproperty vs @property merged_props.update(w.to_dict()) if not w.optional: required.append(w.name) - result = {"type": "object", "properties": merged_props} + result: dict = {"type": "object", "properties": merged_props} if required: result["required"] = required - if self.additional_properties: - result["additionalProperties"] = self.additional_properties.type_dict + if self.additional_properties is not None: + if isinstance(self.additional_properties, bool): + result["additionalProperties"] = self.additional_properties + else: + result["additionalProperties"] = self.additional_properties.type_dict return result diff --git a/tests/core/test_jsonschema_helpers.py b/tests/core/test_jsonschema_helpers.py index 175d0b577..966c31188 100644 --- a/tests/core/test_jsonschema_helpers.py +++ b/tests/core/test_jsonschema_helpers.py @@ -1,7 +1,8 @@ """Test sample sync.""" +from __future__ import annotations + import re -from typing import List import pytest @@ -47,7 +48,7 @@ class ConfigTestTap(Tap): Property("batch_size", IntegerType, default=-1), ).to_dict() - def discover_streams(self) -> List[Stream]: + def discover_streams(self) -> list[Stream]: return [] @@ -291,7 +292,7 @@ def test_array_type(): @pytest.mark.parametrize( - "properties,addtional_properties", + "properties,additional_properties", [ ( [ @@ -311,6 +312,15 @@ def test_array_type(): ], StringType, ), + ( + [ + Property("id", StringType), + Property("email", StringType), + Property("username", StringType), + Property("phone_number", StringType), + ], + False, + ), ( [ Property("id", StringType), @@ -331,6 +341,16 @@ def test_array_type(): ], StringType, ), + ( + [ + Property("id", StringType), + Property("id", StringType), + Property("email", StringType), + Property("username", StringType), + Property("phone_number", StringType), + ], + False, + ), ( [ Property("id", StringType), @@ -349,6 +369,15 @@ def test_array_type(): ], StringType, ), + ( + [ + Property("id", StringType), + Property("email", StringType, True), + Property("username", StringType, True), + Property("phone_number", StringType), + ], + False, + ), ( [ Property("id", StringType), @@ -369,28 +398,49 @@ def test_array_type(): ], StringType, ), + ( + [ + Property("id", StringType), + Property("email", StringType, True), + Property("email", StringType, True), + Property("username", StringType, True), + Property("phone_number", StringType), + ], + False, + ), ], ids=[ - "no requried, no duplicates, no additional properties", - "no requried, no duplicates, additional properties", - "no requried, duplicates, no additional properties", - "no requried, duplicates, additional properties", - "requried, no duplicates, no additional properties", - "requried, no duplicates, additional properties", - "requried, duplicates, no additional properties", - "requried, duplicates, additional properties", + "no required, no duplicates, no additional properties", + "no required, no duplicates, additional properties", + "no required, no duplicates, no additional properties allowed", + "no required, duplicates, no additional properties", + "no required, duplicates, additional properties", + "no required, duplicates, no additional properties allowed", + "required, no duplicates, no additional properties", + "required, no duplicates, additional properties", + "required, no duplicates, no additional properties allowed", + "required, duplicates, no additional properties", + "required, duplicates, additional properties", + "required, duplicates, no additional properties allowed", ], ) -def test_object_type(properties: List[Property], addtional_properties: JSONTypeHelper): +def test_object_type( + properties: list[Property], + additional_properties: JSONTypeHelper | bool, +): merged_property_schemas = { name: schema for p in properties for name, schema in p.to_dict().items() } required = [p.name for p in properties if not p.optional] required_schema = {"required": required} if required else {} - addtional_properties_schema = ( - {"additionalProperties": addtional_properties.type_dict} - if addtional_properties + additional_properties_schema = ( + { + "additionalProperties": additional_properties + if isinstance(additional_properties, bool) + else additional_properties.type_dict + } + if additional_properties is not None else {} ) @@ -398,10 +448,10 @@ def test_object_type(properties: List[Property], addtional_properties: JSONTypeH "type": "object", "properties": merged_property_schemas, **required_schema, - **addtional_properties_schema, + **additional_properties_schema, } - object_type = ObjectType(*properties, additional_properties=addtional_properties) + object_type = ObjectType(*properties, additional_properties=additional_properties) assert object_type.type_dict == expected_json_schema diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 2b87dd75b..66e2850db 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -1,12 +1,15 @@ """Stream tests.""" +from __future__ import annotations + import logging -from typing import Any, Dict, Iterable, List, Optional, cast +from typing import Any, Iterable, cast import pendulum import pytest import requests +from singer_sdk.exceptions import ConfigValidationError from singer_sdk.helpers._classproperty import classproperty from singer_sdk.helpers.jsonpath import _compile_jsonpath from singer_sdk.streams.core import ( @@ -41,7 +44,7 @@ def __init__(self, tap: Tap): """Create a new stream.""" super().__init__(tap, schema=self.schema, name=self.name) - def get_records(self, context: Optional[dict]) -> Iterable[Dict[str, Any]]: + def get_records(self, context: dict | None) -> Iterable[dict[str, Any]]: """Generate records.""" yield {"id": 1, "value": "Egypt"} yield {"id": 2, "value": "Germany"} @@ -78,9 +81,14 @@ class SimpleTestTap(Tap): """Test tap class.""" name = "test-tap" - settings_jsonschema = PropertiesList(Property("start_date", DateTimeType)).to_dict() + config_jsonschema = PropertiesList( + Property("username", StringType, required=True), + Property("password", StringType, required=True), + Property("start_date", DateTimeType), + additional_properties=False, + ).to_dict() - def discover_streams(self) -> List[Stream]: + def discover_streams(self) -> list[Stream]: """List all streams.""" return [SimpleTestStream(self)] @@ -101,7 +109,11 @@ def tap() -> SimpleTestTap: ] } return SimpleTestTap( - config={"start_date": "2021-01-01"}, + config={ + "username": "utest", + "password": "ptest", + "start_date": "2021-01-01", + }, parse_env_config=False, catalog=catalog_dict, ) @@ -214,7 +226,7 @@ def test_stream_starting_timestamp(tap: SimpleTestTap, stream: SimpleTestStream) ], ) def test_jsonpath_rest_stream( - tap: SimpleTestTap, path: str, content: str, result: List[dict] + tap: SimpleTestTap, path: str, content: str, result: list[dict] ): """Validate records are extracted correctly from the API response.""" fake_response = requests.Response() @@ -370,7 +382,7 @@ def test_sync_costs_calculation(tap: SimpleTestTap, caplog): def calculate_test_cost( request: requests.PreparedRequest, response: requests.Response, - context: Optional[Dict], + context: dict | None, ): return {"dim1": 1, "dim2": 2} @@ -387,3 +399,32 @@ def calculate_test_cost( for record in caplog.records: assert record.levelname == "INFO" assert f"Total Sync costs for stream {stream.name}" in record.message + + +@pytest.mark.parametrize( + "config_dict,errors", + [ + ( + {}, + ["'username' is a required property"], + ), + ( + {"username": "utest"}, + ["'password' is a required property"], + ), + ( + {"username": "utest", "password": "ptest", "extra": "not valid"}, + ["Additional properties are not allowed ('extra' was unexpected)"], + ), + ], + ids=[ + "missing_username", + "missing_password", + "extra_property", + ], +) +def test_config_errors(config_dict: dict, errors: list[str]): + with pytest.raises(ConfigValidationError, match="Config validation failed") as exc: + SimpleTestTap(config_dict, validate_config=True) + + assert exc.value.errors == errors From c05087ee0ff0aa8c91b88dba5c10ab4cad539191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Tue, 9 Aug 2022 16:47:00 -0500 Subject: [PATCH 02/14] Get rid of empty warnings list --- singer_sdk/exceptions.py | 3 -- singer_sdk/plugin_base.py | 56 ++++++++++---------------------------- singer_sdk/tap_base.py | 6 ++-- singer_sdk/target_base.py | 18 ++++++++---- tests/core/test_streams.py | 20 +++++++++++++- 5 files changed, 48 insertions(+), 55 deletions(-) diff --git a/singer_sdk/exceptions.py b/singer_sdk/exceptions.py index 773a07253..25385d4d1 100644 --- a/singer_sdk/exceptions.py +++ b/singer_sdk/exceptions.py @@ -13,18 +13,15 @@ def __init__( message: str, *, errors: list[str] | None = None, - warnings: list[str] | None = None, ) -> None: """Initialize a ConfigValidationError. Args: message: A message describing the error. errors: A list of errors which caused the validation error. - warnings: A list of warnings which caused the validation error. """ super().__init__(message) self.errors = errors or [] - self.warnings = warnings or [] class FatalAPIError(Exception): diff --git a/singer_sdk/plugin_base.py b/singer_sdk/plugin_base.py index df93dcf7a..6a0288a51 100644 --- a/singer_sdk/plugin_base.py +++ b/singer_sdk/plugin_base.py @@ -7,21 +7,10 @@ from collections import OrderedDict from pathlib import PurePath from types import MappingProxyType -from typing import ( - Any, - Callable, - Dict, - List, - Mapping, - Optional, - Tuple, - Type, - Union, - cast, -) +from typing import Any, Callable, Dict, List, Mapping, Optional, Type, Union, cast import click -from jsonschema import Draft4Validator, SchemaError, ValidationError +from jsonschema import Draft4Validator from singer_sdk.configuration._dict_config import parse_environment_config from singer_sdk.exceptions import ConfigValidationError @@ -215,35 +204,29 @@ def _is_secret_config(config_key: str) -> bool: """ return is_common_secret_key(config_key) - def _validate_config( - self, raise_errors: bool = True, warnings_as_errors: bool = False - ) -> Tuple[List[str], List[str]]: + def _validate_config(self, raise_errors: bool = True) -> List[str]: """Validate configuration input against the plugin configuration JSON schema. Args: raise_errors: Flag to throw an exception if any validation errors are found. - warnings_as_errors: Flag to throw an exception if any warnings were emitted. Returns: - A tuple of configuration validation warnings and errors. + A list of validation errors. Raises: ConfigValidationError: If raise_errors is True and validation fails. """ - warnings: List[str] = [] - errors: List[str] = [] - log_fn = self.logger.info config_jsonschema = self.config_jsonschema + errors: List[str] = [] + if config_jsonschema: self.append_builtin_config(config_jsonschema) - try: - self.logger.debug( - f"Validating config using jsonschema: {config_jsonschema}" - ) - validator = JSONSchemaValidator(config_jsonschema) - validator.validate(self._config) - except (ValidationError, SchemaError) as ex: - errors.append(str(ex.message)) + self.logger.debug( + f"Validating config using jsonschema: {config_jsonschema}" + ) + validator = JSONSchemaValidator(config_jsonschema) + errors = [error.message for error in validator.iter_errors(self._config)] + if errors: summary = ( f"Config validation failed: {f'; '.join(errors)}\n" @@ -252,18 +235,9 @@ def _validate_config( if raise_errors: raise ConfigValidationError(summary, errors=errors) - log_fn = self.logger.warning - else: - summary = f"Config validation passed with {len(warnings)} warnings." - for warning in warnings: - summary += f"\n{warning}" - if warnings_as_errors and raise_errors and warnings: - raise ConfigValidationError( - f"One or more warnings ocurred during validation: {warnings}", - warnings=warnings, - ) - log_fn(summary) - return warnings, errors + self.logger.warning(summary) + + return errors @classmethod def print_version( diff --git a/singer_sdk/tap_base.py b/singer_sdk/tap_base.py index fd31a2361..0a3200064 100644 --- a/singer_sdk/tap_base.py +++ b/singer_sdk/tap_base.py @@ -497,10 +497,8 @@ def cli( ) except ConfigValidationError as exc: for error in exc.errors: - click.secho(error, fg="red") - for warning in exc.warnings: - click.secho(warning, fg="warning") - raise click.Abort("Configuration is not valid.") + click.secho(error, fg="red", err=True) + raise click.Abort() if discover: tap.run_discovery() diff --git a/singer_sdk/target_base.py b/singer_sdk/target_base.py index dad17f09c..deab5122d 100644 --- a/singer_sdk/target_base.py +++ b/singer_sdk/target_base.py @@ -13,7 +13,7 @@ from joblib import Parallel, delayed, parallel_backend from singer_sdk.cli import common_options -from singer_sdk.exceptions import RecordsWitoutSchemaException +from singer_sdk.exceptions import ConfigValidationError, RecordsWitoutSchemaException from singer_sdk.helpers._classproperty import classproperty from singer_sdk.helpers._compat import final from singer_sdk.helpers.capabilities import CapabilitiesEnum, PluginCapabilities @@ -505,6 +505,7 @@ def cli( Raises: FileNotFoundError: If the config file does not exist. + Abort: If the configuration is not valid. """ if version: cls.print_version() @@ -537,11 +538,16 @@ def cli( config_files.append(Path(config_path)) - target = cls( # type: ignore # Ignore 'type not callable' - config=config_files or None, - parse_env_config=parse_env_config, - validate_config=validate_config, - ) + try: + target = cls( # type: ignore # Ignore 'type not callable' + config=config_files or None, + parse_env_config=parse_env_config, + validate_config=validate_config, + ) + except ConfigValidationError as exc: + for error in exc.errors: + click.secho(error, fg="red", err=True) + raise click.Abort() target.listen(file_input) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 66e2850db..35d67eea9 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -2,12 +2,14 @@ from __future__ import annotations +import json import logging from typing import Any, Iterable, cast import pendulum import pytest import requests +from click.testing import CliRunner from singer_sdk.exceptions import ConfigValidationError from singer_sdk.helpers._classproperty import classproperty @@ -406,7 +408,7 @@ def calculate_test_cost( [ ( {}, - ["'username' is a required property"], + ["'username' is a required property", "'password' is a required property"], ), ( {"username": "utest"}, @@ -428,3 +430,19 @@ def test_config_errors(config_dict: dict, errors: list[str]): SimpleTestTap(config_dict, validate_config=True) assert exc.value.errors == errors + + +def test_cli(tmp_path): + """Test the CLI.""" + runner = CliRunner(mix_stderr=False) + result = runner.invoke(SimpleTestTap.cli, ["--help"]) + assert result.exit_code == 0 + assert "Show this message and exit." in result.output + + config_path = tmp_path / "config.json" + config_path.write_text(json.dumps({})) + result = runner.invoke(SimpleTestTap.cli, ["--config", str(config_path)]) + assert result.exit_code == 1 + assert result.stdout == "" + assert "'username' is a required property" in result.stderr + assert "'password' is a required property" in result.stderr From 03ca8e0187734b112ae95b621ece95ad774e8703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Tue, 9 Aug 2022 17:08:37 -0500 Subject: [PATCH 03/14] Test --discover should not raise error --- 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 35d67eea9..699cd410b 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -446,3 +446,17 @@ def test_cli(tmp_path): assert result.stdout == "" assert "'username' is a required property" in result.stderr assert "'password' is a required property" in result.stderr + + config_path = tmp_path / "config.json" + config_path.write_text(json.dumps({})) + result = runner.invoke( + SimpleTestTap.cli, + [ + "--config", + str(config_path), + "--discover", + ], + ) + assert result.exit_code == 0 + assert "streams" in json.loads(result.stdout) + assert result.stderr == "" From 39af20861afe25f9338ea8f6f40ab618f8338797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Tue, 23 Aug 2022 10:48:30 -0500 Subject: [PATCH 04/14] Tests cleanup --- tests/core/conftest.py | 95 ++++++++++++++++++ tests/core/test_streams.py | 183 +++-------------------------------- tests/core/test_tap_class.py | 71 ++++++++++++++ 3 files changed, 181 insertions(+), 168 deletions(-) create mode 100644 tests/core/test_tap_class.py diff --git a/tests/core/conftest.py b/tests/core/conftest.py index ca3030ce4..158c12d53 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -1,7 +1,102 @@ """Tap, target and stream test fixtures.""" +from __future__ import annotations + +from typing import Any, Iterable + +import pendulum import pytest +from singer_sdk import Stream, Tap +from singer_sdk.typing import ( + DateTimeType, + IntegerType, + PropertiesList, + Property, + StringType, +) + + +class SimpleTestStream(Stream): + """Test stream class.""" + + name = "test" + schema = PropertiesList( + Property("id", IntegerType, required=True), + Property("value", StringType, required=True), + Property("updatedAt", DateTimeType, required=True), + ).to_dict() + replication_key = "updatedAt" + + def __init__(self, tap: Tap): + """Create a new stream.""" + super().__init__(tap, schema=self.schema, name=self.name) + + def get_records(self, context: dict | None) -> Iterable[dict[str, Any]]: + """Generate records.""" + yield {"id": 1, "value": "Egypt"} + yield {"id": 2, "value": "Germany"} + yield {"id": 3, "value": "India"} + + +class UnixTimestampIncrementalStream(SimpleTestStream): + name = "unix_ts" + schema = PropertiesList( + Property("id", IntegerType, required=True), + Property("value", StringType, required=True), + Property("updatedAt", IntegerType, required=True), + ).to_dict() + replication_key = "updatedAt" + + +class UnixTimestampIncrementalStream2(UnixTimestampIncrementalStream): + name = "unix_ts_override" + + def compare_start_date(self, value: str, start_date_value: str) -> str: + """Compare a value to a start date value.""" + + start_timestamp = pendulum.parse(start_date_value).format("X") + return max(value, start_timestamp, key=float) + + +class SimpleTestTap(Tap): + """Test tap class.""" + + name = "test-tap" + config_jsonschema = PropertiesList( + Property("username", StringType, required=True), + Property("password", StringType, required=True), + Property("start_date", DateTimeType), + additional_properties=False, + ).to_dict() + + def discover_streams(self) -> list[Stream]: + """List all streams.""" + return [ + SimpleTestStream(self), + UnixTimestampIncrementalStream(self), + UnixTimestampIncrementalStream2(self), + ] + + +@pytest.fixture +def tap_class(): + """Return the tap class.""" + return SimpleTestTap + + +@pytest.fixture +def tap() -> SimpleTestTap: + """Tap instance.""" + return SimpleTestTap( + config={ + "username": "utest", + "password": "ptest", + "start_date": "2021-01-01", + }, + parse_env_config=False, + ) + @pytest.fixture def csv_config(outdir: str) -> dict: diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index 2fe4d8b5e..2861b5f74 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -2,78 +2,25 @@ from __future__ import annotations -import json import logging -from typing import Any, Iterable, cast +from typing import TYPE_CHECKING, Any import pendulum import pytest import requests -from click.testing import CliRunner -from singer_sdk.exceptions import ConfigValidationError from singer_sdk.helpers._classproperty import classproperty -from singer_sdk.helpers._singer import Catalog, CatalogEntry, MetadataMapping +from singer_sdk.helpers._singer import Catalog, MetadataMapping from singer_sdk.helpers.jsonpath import _compile_jsonpath -from singer_sdk.streams.core import ( - REPLICATION_FULL_TABLE, - REPLICATION_INCREMENTAL, - Stream, -) +from singer_sdk.streams.core import REPLICATION_FULL_TABLE, REPLICATION_INCREMENTAL from singer_sdk.streams.graphql import GraphQLStream from singer_sdk.streams.rest import RESTStream -from singer_sdk.tap_base import Tap -from singer_sdk.typing import ( - DateTimeType, - IntegerType, - PropertiesList, - Property, - StringType, -) +from singer_sdk.typing import IntegerType, PropertiesList, Property, StringType CONFIG_START_DATE = "2021-01-01" - -class SimpleTestStream(Stream): - """Test stream class.""" - - name = "test" - schema = PropertiesList( - Property("id", IntegerType, required=True), - Property("value", StringType, required=True), - Property("updatedAt", DateTimeType, required=True), - ).to_dict() - replication_key = "updatedAt" - - def __init__(self, tap: Tap): - """Create a new stream.""" - super().__init__(tap, schema=self.schema, name=self.name) - - def get_records(self, context: dict | None) -> Iterable[dict[str, Any]]: - """Generate records.""" - yield {"id": 1, "value": "Egypt"} - yield {"id": 2, "value": "Germany"} - yield {"id": 3, "value": "India"} - - -class UnixTimestampIncrementalStream(SimpleTestStream): - name = "unix_ts" - schema = PropertiesList( - Property("id", IntegerType, required=True), - Property("value", StringType, required=True), - Property("updatedAt", IntegerType, required=True), - ).to_dict() - replication_key = "updatedAt" - - -class UnixTimestampIncrementalStream2(UnixTimestampIncrementalStream): - name = "unix_ts_override" - - def compare_start_date(self, value: str, start_date_value: str) -> str: - """Compare a value to a start date value.""" - - start_timestamp = pendulum.parse(start_date_value).format("X") - return max(value, start_timestamp, key=float) +if TYPE_CHECKING: + from singer_sdk import Stream, Tap class RestTestStream(RESTStream): @@ -102,52 +49,13 @@ class GraphqlTestStream(GraphQLStream): replication_key = "updatedAt" -class SimpleTestTap(Tap): - """Test tap class.""" - - name = "test-tap" - config_jsonschema = PropertiesList( - Property("username", StringType, required=True), - Property("password", StringType, required=True), - Property("start_date", DateTimeType), - additional_properties=False, - ).to_dict() - - def discover_streams(self) -> list[Stream]: - """List all streams.""" - return [ - SimpleTestStream(self), - UnixTimestampIncrementalStream(self), - UnixTimestampIncrementalStream2(self), - ] - - @pytest.fixture -def tap() -> SimpleTestTap: - """Tap instance.""" - return SimpleTestTap( - config={ - "username": "utest", - "password": "ptest", - "start_date": "2021-01-01", - }, - parse_env_config=False, - ) - - -@pytest.fixture -def stream(tap: SimpleTestTap) -> SimpleTestStream: +def stream(tap): """Create a new stream instance.""" - return cast(SimpleTestStream, tap.load_streams()[0]) + return tap.load_streams()[0] -@pytest.fixture -def unix_timestamp_stream(tap: SimpleTestTap) -> UnixTimestampIncrementalStream: - """Create a new stream instance.""" - return cast(UnixTimestampIncrementalStream, tap.load_streams()[1]) - - -def test_stream_apply_catalog(tap: SimpleTestTap, stream: SimpleTestStream): +def test_stream_apply_catalog(stream: Stream): """Applying a catalog to a stream should overwrite fields.""" assert stream.primary_keys == [] assert stream.replication_key == "updatedAt" @@ -238,7 +146,7 @@ def test_stream_apply_catalog(tap: SimpleTestTap, stream: SimpleTestStream): ], ) def test_stream_starting_timestamp( - tap: SimpleTestTap, + tap: Tap, stream_name: str, bookmark_value: str, expected_starting_value: Any, @@ -322,9 +230,7 @@ def test_stream_starting_timestamp( "nested_values", ], ) -def test_jsonpath_rest_stream( - tap: SimpleTestTap, path: str, content: str, result: list[dict] -): +def test_jsonpath_rest_stream(tap: Tap, path: str, content: str, result: list[dict]): """Validate records are extracted correctly from the API response.""" fake_response = requests.Response() fake_response._content = str.encode(content) @@ -337,7 +243,7 @@ def test_jsonpath_rest_stream( assert list(rows) == result -def test_jsonpath_graphql_stream_default(tap: SimpleTestTap): +def test_jsonpath_graphql_stream_default(tap: Tap): """Validate graphql JSONPath, defaults to the stream name.""" content = """{ "data": { @@ -357,7 +263,7 @@ def test_jsonpath_graphql_stream_default(tap: SimpleTestTap): assert list(rows) == [{"id": 1, "value": "abc"}, {"id": 2, "value": "def"}] -def test_jsonpath_graphql_stream_override(tap: SimpleTestTap): +def test_jsonpath_graphql_stream_override(tap: Tap): """Validate graphql jsonpath can be updated.""" content = """[ {"id": 1, "value": "abc"}, @@ -444,7 +350,7 @@ def records_jsonpath(cls): ], ) def test_next_page_token_jsonpath( - tap: SimpleTestTap, path: str, content: str, headers: dict, result: str + tap: Tap, path: str, content: str, headers: dict, result: str ): """Validate pagination token is extracted correctly from API response.""" fake_response = requests.Response() @@ -469,7 +375,7 @@ def test_cached_jsonpath(): assert recompiled is compiled -def test_sync_costs_calculation(tap: SimpleTestTap, caplog): +def test_sync_costs_calculation(tap: Tap, caplog): """Test sync costs are added up correctly.""" fake_request = requests.PreparedRequest() fake_response = requests.Response() @@ -496,62 +402,3 @@ def calculate_test_cost( for record in caplog.records: assert record.levelname == "INFO" assert f"Total Sync costs for stream {stream.name}" in record.message - - -@pytest.mark.parametrize( - "config_dict,errors", - [ - ( - {}, - ["'username' is a required property", "'password' is a required property"], - ), - ( - {"username": "utest"}, - ["'password' is a required property"], - ), - ( - {"username": "utest", "password": "ptest", "extra": "not valid"}, - ["Additional properties are not allowed ('extra' was unexpected)"], - ), - ], - ids=[ - "missing_username", - "missing_password", - "extra_property", - ], -) -def test_config_errors(config_dict: dict, errors: list[str]): - with pytest.raises(ConfigValidationError, match="Config validation failed") as exc: - SimpleTestTap(config_dict, validate_config=True) - - assert exc.value.errors == errors - - -def test_cli(tmp_path): - """Test the CLI.""" - runner = CliRunner(mix_stderr=False) - result = runner.invoke(SimpleTestTap.cli, ["--help"]) - assert result.exit_code == 0 - assert "Show this message and exit." in result.output - - config_path = tmp_path / "config.json" - config_path.write_text(json.dumps({})) - result = runner.invoke(SimpleTestTap.cli, ["--config", str(config_path)]) - assert result.exit_code == 1 - assert result.stdout == "" - assert "'username' is a required property" in result.stderr - assert "'password' is a required property" in result.stderr - - config_path = tmp_path / "config.json" - config_path.write_text(json.dumps({})) - result = runner.invoke( - SimpleTestTap.cli, - [ - "--config", - str(config_path), - "--discover", - ], - ) - assert result.exit_code == 0 - assert "streams" in json.loads(result.stdout) - assert result.stderr == "" diff --git a/tests/core/test_tap_class.py b/tests/core/test_tap_class.py new file mode 100644 index 000000000..08b017f5b --- /dev/null +++ b/tests/core/test_tap_class.py @@ -0,0 +1,71 @@ +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest +from click.testing import CliRunner + +from singer_sdk.exceptions import ConfigValidationError + +if TYPE_CHECKING: + from singer_sdk import Tap + + +@pytest.mark.parametrize( + "config_dict,errors", + [ + ( + {}, + ["'username' is a required property", "'password' is a required property"], + ), + ( + {"username": "utest"}, + ["'password' is a required property"], + ), + ( + {"username": "utest", "password": "ptest", "extra": "not valid"}, + ["Additional properties are not allowed ('extra' was unexpected)"], + ), + ], + ids=[ + "missing_username", + "missing_password", + "extra_property", + ], +) +def test_config_errors(tap_class: type[Tap], config_dict: dict, errors: list[str]): + with pytest.raises(ConfigValidationError, match="Config validation failed") as exc: + tap_class(config_dict, validate_config=True) + + assert exc.value.errors == errors + + +def test_cli(tap_class: type[Tap], tmp_path): + """Test the CLI.""" + runner = CliRunner(mix_stderr=False) + result = runner.invoke(tap_class.cli, ["--help"]) + assert result.exit_code == 0 + assert "Show this message and exit." in result.output + + config_path = tmp_path / "config.json" + config_path.write_text(json.dumps({})) + result = runner.invoke(tap_class.cli, ["--config", str(config_path)]) + assert result.exit_code == 1 + assert result.stdout == "" + assert "'username' is a required property" in result.stderr + assert "'password' is a required property" in result.stderr + + config_path = tmp_path / "config.json" + config_path.write_text(json.dumps({})) + result = runner.invoke( + tap_class.cli, + [ + "--config", + str(config_path), + "--discover", + ], + ) + assert result.exit_code == 0 + assert "streams" in json.loads(result.stdout) + assert result.stderr == "" From 7e1d10a90cd229122fb60d86d400d446be533f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Tue, 23 Aug 2022 11:09:56 -0500 Subject: [PATCH 05/14] Add target class init test --- samples/sample_tap_sqlite/__init__.py | 1 + samples/sample_target_sqlite/__init__.py | 1 + tests/core/test_target_class.py | 50 ++++++++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 tests/core/test_target_class.py diff --git a/samples/sample_tap_sqlite/__init__.py b/samples/sample_tap_sqlite/__init__.py index 5f92b85d0..ea93796ee 100644 --- a/samples/sample_tap_sqlite/__init__.py +++ b/samples/sample_tap_sqlite/__init__.py @@ -60,6 +60,7 @@ class SQLiteTap(SQLTap): DB_PATH_CONFIG, th.StringType, description="The path to your SQLite database file(s).", + required=True, ) ).to_dict() diff --git a/samples/sample_target_sqlite/__init__.py b/samples/sample_target_sqlite/__init__.py index d4e4372bd..1440f6eef 100644 --- a/samples/sample_target_sqlite/__init__.py +++ b/samples/sample_target_sqlite/__init__.py @@ -66,6 +66,7 @@ class SQLiteTarget(SQLTarget): DB_PATH_CONFIG, th.StringType, description="The path to your SQLite database file(s).", + required=True, ) ).to_dict() diff --git a/tests/core/test_target_class.py b/tests/core/test_target_class.py new file mode 100644 index 000000000..ee793651d --- /dev/null +++ b/tests/core/test_target_class.py @@ -0,0 +1,50 @@ +from __future__ import annotations + +import json +from contextlib import nullcontext + +import pytest +from click.testing import CliRunner + +from samples.sample_target_sqlite import SQLiteTarget +from singer_sdk.exceptions import ConfigValidationError + + +@pytest.mark.parametrize( + "config_dict,expectation,errors", + [ + pytest.param( + {}, + pytest.raises(ConfigValidationError, match="Config validation failed"), + ["'path_to_db' is a required property"], + id="missing_path_to_db", + ), + pytest.param( + {"path_to_db": "sqlite://test.db"}, + nullcontext(), + [], + id="valid_config", + ), + ], +) +def test_config_errors(config_dict: dict, expectation, errors: list[str]): + with expectation as exc: + SQLiteTarget(config_dict, validate_config=True) + + if isinstance(exc, pytest.ExceptionInfo): + assert exc.value.errors == errors + + +def test_cli(tmp_path): + """Test the CLI.""" + runner = CliRunner(mix_stderr=False) + result = runner.invoke(SQLiteTarget.cli, ["--help"]) + assert result.exit_code == 0 + assert "Show this message and exit." in result.output + + config_path = tmp_path / "config.json" + config_path.write_text(json.dumps({})) + result = runner.invoke(SQLiteTarget.cli, ["--config", str(config_path)]) + assert result.exit_code == 1 + assert result.stdout == "" + assert "'path_to_db' is a required property" in result.stderr From 63b8e1386ac732f8c420dee60cb5a94c5b688dc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Wed, 24 Aug 2022 22:36:46 -0500 Subject: [PATCH 06/14] Add JSONTypeHelper.to_json wrapper --- singer_sdk/typing.py | 89 ++++++++++++++++++++++++++- tests/core/test_jsonschema_helpers.py | 43 +++++++++++++ 2 files changed, 130 insertions(+), 2 deletions(-) diff --git a/singer_sdk/typing.py b/singer_sdk/typing.py index 9de754b68..b0f649e14 100644 --- a/singer_sdk/typing.py +++ b/singer_sdk/typing.py @@ -41,8 +41,9 @@ from __future__ import annotations +import json import sys -from typing import Generic, Mapping, TypeVar, Union, cast +from typing import Any, Generic, Mapping, TypeVar, Union, cast import sqlalchemy from jsonschema import validators @@ -143,6 +144,17 @@ def to_dict(self) -> dict: """ return cast(dict, self.type_dict) + def to_json(self, **kwargs: Any) -> str: + """Convert to JSON. + + Args: + kwargs: Additional keyword arguments to pass to json.dumps(). + + Returns: + A JSON string describing the object. + """ + return json.dumps(self.to_dict(), **kwargs) + class StringType(JSONTypeHelper): """String type.""" @@ -418,7 +430,80 @@ def __init__( Args: properties: Zero or more attributes for this JSON object. additional_properties: A schema to match against unnamed properties in - this object or a boolean indicating if extra properties are allowed. + this object, or a boolean indicating if extra properties are allowed. + + Examples: + >>> t = ObjectType( + ... Property("name", StringType, required=True), + ... Property("age", IntegerType), + ... Property("height", NumberType), + ... additional_properties=False, + ... ) + >>> print(t.to_json(indent=2)) + { + "type": "object", + "properties": { + "name": { + "type": [ + "string" + ] + }, + "age": { + "type": [ + "integer", + "null" + ] + }, + "height": { + "type": [ + "number", + "null" + ] + } + }, + "required": [ + "name" + ], + "additionalProperties": false + } + + >>> t = ObjectType( + ... Property("name", StringType, required=True), + ... Property("age", IntegerType), + ... Property("height", NumberType), + ... additional_properties=StringType, + ... ) + >>> print(t.to_json(indent=2)) + { + "type": "object", + "properties": { + "name": { + "type": [ + "string" + ] + }, + "age": { + "type": [ + "integer", + "null" + ] + }, + "height": { + "type": [ + "number", + "null" + ] + } + }, + "required": [ + "name" + ], + "additionalProperties": { + "type": [ + "string" + ] + } + } """ self.wrapped: list[Property] = list(properties) self.additional_properties = additional_properties diff --git a/tests/core/test_jsonschema_helpers.py b/tests/core/test_jsonschema_helpers.py index 966c31188..75638389f 100644 --- a/tests/core/test_jsonschema_helpers.py +++ b/tests/core/test_jsonschema_helpers.py @@ -3,6 +3,7 @@ from __future__ import annotations import re +from textwrap import dedent import pytest @@ -52,6 +53,48 @@ def discover_streams(self) -> list[Stream]: return [] +def test_to_json(): + schema = PropertiesList( + Property( + "test_property", + StringType, + description="A test property", + required=True, + ), + Property( + "test_property_2", + StringType, + description="A test property", + ), + additional_properties=False, + ) + assert schema.to_json(indent=4) == dedent( + """\ + { + "type": "object", + "properties": { + "test_property": { + "type": [ + "string" + ], + "description": "A test property" + }, + "test_property_2": { + "type": [ + "string", + "null" + ], + "description": "A test property" + } + }, + "required": [ + "test_property" + ], + "additionalProperties": false + }""" + ) + + def test_nested_complex_objects(): test1a = Property( "Datasets", From f3ccdc39c4487aa2ee7792156fbc863c1e5f2306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Tue, 13 Sep 2022 23:58:34 -0500 Subject: [PATCH 07/14] Test mapper class configuration --- singer_sdk/mapper_base.py | 15 +++++++--- singer_sdk/tap_base.py | 4 +-- singer_sdk/target_base.py | 3 +- tests/core/test_mapper_class.py | 50 +++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 tests/core/test_mapper_class.py diff --git a/singer_sdk/mapper_base.py b/singer_sdk/mapper_base.py index c09d39255..856ee4f79 100644 --- a/singer_sdk/mapper_base.py +++ b/singer_sdk/mapper_base.py @@ -1,6 +1,7 @@ """Abstract base class for stream mapper plugins.""" import abc +import sys from io import FileIO from typing import Callable, Iterable, List, Tuple @@ -9,6 +10,7 @@ from singer_sdk.cli import common_options from singer_sdk.configuration._dict_config import merge_config_sources +from singer_sdk.exceptions import ConfigValidationError from singer_sdk.helpers._classproperty import classproperty from singer_sdk.helpers.capabilities import CapabilitiesEnum, PluginCapabilities from singer_sdk.io_base import SingerReader @@ -143,10 +145,15 @@ def cli( cls._env_prefix, ) - mapper = cls( # type: ignore # Ignore 'type not callable' - config=config_dict, - validate_config=validate_config, - ) + try: + mapper = cls( # type: ignore # Ignore 'type not callable' + config=config_dict, + validate_config=validate_config, + ) + except ConfigValidationError as exc: + for error in exc.errors: + click.secho(error, fg="red", err=True) + sys.exit(1) if about: mapper.print_about(format) diff --git a/singer_sdk/tap_base.py b/singer_sdk/tap_base.py index 0a3200064..698f5b3aa 100644 --- a/singer_sdk/tap_base.py +++ b/singer_sdk/tap_base.py @@ -2,6 +2,7 @@ import abc import json +import sys from enum import Enum from pathlib import Path, PurePath from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union, cast @@ -453,7 +454,6 @@ def cli( Raises: FileNotFoundError: If the config file does not exist. - Abort: If the configuration is not valid. """ if version: cls.print_version() @@ -498,7 +498,7 @@ def cli( except ConfigValidationError as exc: for error in exc.errors: click.secho(error, fg="red", err=True) - raise click.Abort() + sys.exit(1) if discover: tap.run_discovery() diff --git a/singer_sdk/target_base.py b/singer_sdk/target_base.py index 3da2945bf..8e23bb473 100644 --- a/singer_sdk/target_base.py +++ b/singer_sdk/target_base.py @@ -506,7 +506,6 @@ def cli( Raises: FileNotFoundError: If the config file does not exist. - Abort: If the configuration is not valid. """ if version: cls.print_version() @@ -548,7 +547,7 @@ def cli( except ConfigValidationError as exc: for error in exc.errors: click.secho(error, fg="red", err=True) - raise click.Abort() + sys.exit(1) target.listen(file_input) diff --git a/tests/core/test_mapper_class.py b/tests/core/test_mapper_class.py new file mode 100644 index 000000000..cdc028ad2 --- /dev/null +++ b/tests/core/test_mapper_class.py @@ -0,0 +1,50 @@ +from __future__ import annotations + +import json +from contextlib import nullcontext + +import pytest +from click.testing import CliRunner + +from samples.sample_mapper.mapper import StreamTransform +from singer_sdk.exceptions import ConfigValidationError + + +@pytest.mark.parametrize( + "config_dict,expectation,errors", + [ + pytest.param( + {}, + pytest.raises(ConfigValidationError, match="Config validation failed"), + ["'stream_maps' is a required property"], + id="missing_stream_maps", + ), + pytest.param( + {"stream_maps": {}}, + nullcontext(), + [], + id="valid_config", + ), + ], +) +def test_config_errors(config_dict: dict, expectation, errors: list[str]): + with expectation as exc: + StreamTransform(config_dict, validate_config=True) + + if isinstance(exc, pytest.ExceptionInfo): + assert exc.value.errors == errors + + +def test_cli(tmp_path): + """Test the CLI.""" + runner = CliRunner(mix_stderr=False) + result = runner.invoke(StreamTransform.cli, ["--help"]) + assert result.exit_code == 0 + assert "Show this message and exit." in result.output + + config_path = tmp_path / "config.json" + config_path.write_text(json.dumps({})) + result = runner.invoke(StreamTransform.cli, ["--config", str(config_path)]) + assert result.exit_code == 1 + assert result.stdout == "" + assert "'stream_maps' is a required property" in result.stderr From 03d1caeeb6d6563e876b7f68eb5dfcdd68f4c4bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Wed, 15 Feb 2023 20:25:14 -0600 Subject: [PATCH 08/14] Split the CLI tests --- tests/core/test_mapper_class.py | 8 ++++-- tests/core/test_tap_class.py | 46 ++++++++++++++++++++++++--------- tests/core/test_target_class.py | 6 ++++- 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/tests/core/test_mapper_class.py b/tests/core/test_mapper_class.py index cdc028ad2..496f00a82 100644 --- a/tests/core/test_mapper_class.py +++ b/tests/core/test_mapper_class.py @@ -35,13 +35,17 @@ def test_config_errors(config_dict: dict, expectation, errors: list[str]): assert exc.value.errors == errors -def test_cli(tmp_path): - """Test the CLI.""" +def test_cli_help(): + """Test the CLI help message.""" runner = CliRunner(mix_stderr=False) result = runner.invoke(StreamTransform.cli, ["--help"]) assert result.exit_code == 0 assert "Show this message and exit." in result.output + +def test_cli_config_validation(tmp_path): + """Test the CLI config validation.""" + runner = CliRunner(mix_stderr=False) config_path = tmp_path / "config.json" config_path.write_text(json.dumps({})) result = runner.invoke(StreamTransform.cli, ["--config", str(config_path)]) diff --git a/tests/core/test_tap_class.py b/tests/core/test_tap_class.py index 08b017f5b..0cd601f84 100644 --- a/tests/core/test_tap_class.py +++ b/tests/core/test_tap_class.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +from contextlib import nullcontext from typing import TYPE_CHECKING import pytest @@ -13,32 +14,45 @@ @pytest.mark.parametrize( - "config_dict,errors", + "config_dict,expectation,errors", [ - ( + pytest.param( {}, + pytest.raises(ConfigValidationError, match="Config validation failed"), ["'username' is a required property", "'password' is a required property"], + id="missing_username_and_password", ), - ( + pytest.param( {"username": "utest"}, + pytest.raises(ConfigValidationError, match="Config validation failed"), ["'password' is a required property"], + id="missing_password", ), - ( + pytest.param( {"username": "utest", "password": "ptest", "extra": "not valid"}, + pytest.raises(ConfigValidationError, match="Config validation failed"), ["Additional properties are not allowed ('extra' was unexpected)"], + id="extra_property", + ), + pytest.param( + {"username": "utest", "password": "ptest"}, + nullcontext(), + [], + id="valid_config", ), - ], - ids=[ - "missing_username", - "missing_password", - "extra_property", ], ) -def test_config_errors(tap_class: type[Tap], config_dict: dict, errors: list[str]): - with pytest.raises(ConfigValidationError, match="Config validation failed") as exc: +def test_config_errors( + tap_class: type[Tap], + config_dict: dict, + expectation, + errors: list[str], +): + with expectation as exc: tap_class(config_dict, validate_config=True) - assert exc.value.errors == errors + if isinstance(exc, pytest.ExceptionInfo): + assert exc.value.errors == errors def test_cli(tap_class: type[Tap], tmp_path): @@ -48,6 +62,10 @@ def test_cli(tap_class: type[Tap], tmp_path): assert result.exit_code == 0 assert "Show this message and exit." in result.output + +def test_cli_config_validation(tap_class: type[Tap], tmp_path): + """Test the CLI config validation.""" + runner = CliRunner(mix_stderr=False) config_path = tmp_path / "config.json" config_path.write_text(json.dumps({})) result = runner.invoke(tap_class.cli, ["--config", str(config_path)]) @@ -56,6 +74,10 @@ def test_cli(tap_class: type[Tap], tmp_path): assert "'username' is a required property" in result.stderr assert "'password' is a required property" in result.stderr + +def test_cli_discover(tap_class: type[Tap], tmp_path): + """Test the CLI discover command.""" + runner = CliRunner(mix_stderr=False) config_path = tmp_path / "config.json" config_path.write_text(json.dumps({})) result = runner.invoke( diff --git a/tests/core/test_target_class.py b/tests/core/test_target_class.py index ee793651d..56d89f1f9 100644 --- a/tests/core/test_target_class.py +++ b/tests/core/test_target_class.py @@ -35,13 +35,17 @@ def test_config_errors(config_dict: dict, expectation, errors: list[str]): assert exc.value.errors == errors -def test_cli(tmp_path): +def test_cli(): """Test the CLI.""" runner = CliRunner(mix_stderr=False) result = runner.invoke(SQLiteTarget.cli, ["--help"]) assert result.exit_code == 0 assert "Show this message and exit." in result.output + +def test_cli_config_validation(tmp_path): + """Test the CLI config validation.""" + runner = CliRunner(mix_stderr=False) config_path = tmp_path / "config.json" config_path.write_text(json.dumps({})) result = runner.invoke(SQLiteTarget.cli, ["--config", str(config_path)]) From 5b8d67cdcf6ad979aee691344c14aa305b8c7bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Tue, 25 Apr 2023 14:39:06 -0600 Subject: [PATCH 09/14] Get rid of color --- singer_sdk/mapper_base.py | 2 +- singer_sdk/tap_base.py | 2 +- singer_sdk/target_base.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/singer_sdk/mapper_base.py b/singer_sdk/mapper_base.py index fc7e27e21..ebe1caf4f 100644 --- a/singer_sdk/mapper_base.py +++ b/singer_sdk/mapper_base.py @@ -166,7 +166,7 @@ def cli( ) except ConfigValidationError as exc: for error in exc.errors: - click.secho(error, fg="red", err=True) + click.secho(error, err=True) sys.exit(1) if about: diff --git a/singer_sdk/tap_base.py b/singer_sdk/tap_base.py index d9b4730fc..e1e7137ca 100644 --- a/singer_sdk/tap_base.py +++ b/singer_sdk/tap_base.py @@ -529,7 +529,7 @@ def cli( ) except ConfigValidationError as exc: for error in exc.errors: - click.secho(error, fg="red", err=True) + click.secho(error, err=True) sys.exit(1) if discover: diff --git a/singer_sdk/target_base.py b/singer_sdk/target_base.py index 1038b96df..ddbf48e04 100644 --- a/singer_sdk/target_base.py +++ b/singer_sdk/target_base.py @@ -576,7 +576,7 @@ def cli( ) except ConfigValidationError as exc: for error in exc.errors: - click.secho(error, fg="red", err=True) + click.secho(error, err=True) sys.exit(1) target.listen(file_input) From 624a658c62d88e18b6a3531d08b946e5245ac418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez=20Mondrag=C3=B3n?= Date: Tue, 18 Jul 2023 16:31:34 -0600 Subject: [PATCH 10/14] Address lint issues --- tests/core/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/core/conftest.py b/tests/core/conftest.py index f1ff83b50..06355ccfe 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import Any, Iterable +import typing as t import pendulum import pytest @@ -35,7 +35,7 @@ def __init__(self, tap: Tap): def get_records( self, context: dict | None, # noqa: ARG002 - ) -> Iterable[dict[str, Any]]: + ) -> t.Iterable[dict[str, t.Any]]: """Generate records.""" yield {"id": 1, "value": "Egypt"} yield {"id": 2, "value": "Germany"} From 2852dbeb802ca19f5045c779006826864d95d573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez=20Mondrag=C3=B3n?= Date: Tue, 18 Jul 2023 16:35:00 -0600 Subject: [PATCH 11/14] Do not validate config in selection test --- tests/core/test_streams.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index c4999e867..48bcc69ca 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -503,10 +503,7 @@ def discover_streams(self): return [SelectedStream(self), UnselectedStream(self)] # Check that the selected stream is selected - tap = MyTap( - config={"username": "dummy", "password": "s3cr3t"}, - catalog=input_catalog, - ) + tap = MyTap(config=None, catalog=input_catalog, validate_config=False) assert all( tap.streams[stream].selected is selection[stream] for stream in selection ) From e8443f815abba83cb5ddb826228a527cefb8054f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez=20Mondrag=C3=B3n?= Date: Thu, 20 Jul 2023 13:48:15 -0600 Subject: [PATCH 12/14] Revert unnecessary change --- singer_sdk/tap_base.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/singer_sdk/tap_base.py b/singer_sdk/tap_base.py index 3eadc7710..08ce6f3ad 100644 --- a/singer_sdk/tap_base.py +++ b/singer_sdk/tap_base.py @@ -13,10 +13,7 @@ from singer_sdk._singerlib import Catalog, StateMessage, write_message from singer_sdk.configuration._dict_config import merge_missing_config_jsonschema -from singer_sdk.exceptions import ( - AbortedSyncFailedException, - AbortedSyncPausedException, -) +from singer_sdk.exceptions import AbortedSyncFailedException, AbortedSyncPausedException from singer_sdk.helpers import _state from singer_sdk.helpers._classproperty import classproperty from singer_sdk.helpers._compat import final From 96b819318b28a707038f2999340ecc53fcd5573e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez=20Mondrag=C3=B3n?= Date: Fri, 17 Nov 2023 15:45:02 -0600 Subject: [PATCH 13/14] Address lint issues --- tests/core/test_streams.py | 2 ++ tests/samples/test_target_sqlite.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/core/test_streams.py b/tests/core/test_streams.py index dbdf35d79..8a415e55d 100644 --- a/tests/core/test_streams.py +++ b/tests/core/test_streams.py @@ -20,11 +20,13 @@ from singer_sdk.streams.graphql import GraphQLStream from singer_sdk.streams.rest import RESTStream from singer_sdk.typing import IntegerType, PropertiesList, Property, StringType +from tests.core.conftest import SimpleTestStream CONFIG_START_DATE = "2021-01-01" if t.TYPE_CHECKING: from singer_sdk import Stream, Tap + from tests.core.conftest import SimpleTestTap class RestTestStream(RESTStream): diff --git a/tests/samples/test_target_sqlite.py b/tests/samples/test_target_sqlite.py index 727b760ba..a66805a09 100644 --- a/tests/samples/test_target_sqlite.py +++ b/tests/samples/test_target_sqlite.py @@ -36,7 +36,7 @@ def path_to_target_db(tmp_path: Path) -> Path: @pytest.fixture -def sqlite_target_test_config(path_to_target_db: str) -> dict: +def sqlite_target_test_config(path_to_target_db: Path) -> dict: """Get configuration dictionary for target-csv.""" return {"path_to_db": str(path_to_target_db)} From 93e29c020791c146131e2703e36439ad837242db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez=20Mondrag=C3=B3n?= Date: Fri, 17 Nov 2023 15:56:35 -0600 Subject: [PATCH 14/14] Use logger instead of click.secho --- singer_sdk/plugin_base.py | 25 +++++++++++++++++++++---- tests/core/test_tap_class.py | 1 - 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/singer_sdk/plugin_base.py b/singer_sdk/plugin_base.py index 747f5908e..b4e82296b 100644 --- a/singer_sdk/plugin_base.py +++ b/singer_sdk/plugin_base.py @@ -75,6 +75,22 @@ def __init__(self) -> None: class SingerCommand(click.Command): """Custom click command class for Singer packages.""" + def __init__( + self, + *args: t.Any, + logger: logging.Logger, + **kwargs: t.Any, + ) -> None: + """Initialize the command. + + Args: + *args: Positional `click.Command` arguments. + logger: A logger instance. + **kwargs: Keyword `click.Command` arguments. + """ + super().__init__(*args, **kwargs) + self.logger = logger + def invoke(self, ctx: click.Context) -> t.Any: # noqa: ANN401 """Invoke the command, capturing warnings and logging them. @@ -89,7 +105,7 @@ def invoke(self, ctx: click.Context) -> t.Any: # noqa: ANN401 return super().invoke(ctx) except ConfigValidationError as exc: for error in exc.errors: - click.secho(f"Error: {error}", err=True) + self.logger.error("Config validation error: %s", error) sys.exit(1) @@ -171,12 +187,12 @@ def __init__( if self._is_secret_config(k): config_dict[k] = SecretString(v) self._config = config_dict - self._validate_config(raise_errors=validate_config) - self._mapper: PluginMapper | None = None - metrics._setup_logging(self.config) self.metrics_logger = metrics.get_metrics_logger() + self._validate_config(raise_errors=validate_config) + self._mapper: PluginMapper | None = None + # Initialization timestamp self.__initialized_at = int(time.time() * 1000) @@ -601,6 +617,7 @@ def get_singer_command(cls: type[PluginBase]) -> click.Command: is_eager=True, ), ], + logger=cls.logger, ) @plugin_cli diff --git a/tests/core/test_tap_class.py b/tests/core/test_tap_class.py index b57602a4c..93015fbb1 100644 --- a/tests/core/test_tap_class.py +++ b/tests/core/test_tap_class.py @@ -90,4 +90,3 @@ def test_cli_discover(tap_class: type[Tap], tmp_path): ) assert result.exit_code == 0 assert "streams" in json.loads(result.stdout) - assert not result.stderr