From 8eb575dba26f6d39c3872b0a118d839964da051e Mon Sep 17 00:00:00 2001 From: Jesufemi-O Date: Sun, 2 Aug 2026 15:20:55 -0400 Subject: [PATCH 1/4] =?UTF-8?q?feat(t3-3):=20SourceFactory.collect=5Fconfi?= =?UTF-8?q?g()=20=E2=80=94=20spec-driven,=20no=20per-source=20branching?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interactive mode: prompts credentials with hide_input and env-var default; prompts config_fields respecting required/optional. Non-interactive: reads from flags dict, falls back to env-var refs for credentials, raises SystemExit(1) for missing required config fields. Closes #102 --- src/tycoon/ingestion/factory.py | 85 ++++++++++++++++++ tests/test_factory.py | 147 ++++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 src/tycoon/ingestion/factory.py create mode 100644 tests/test_factory.py diff --git a/src/tycoon/ingestion/factory.py b/src/tycoon/ingestion/factory.py new file mode 100644 index 0000000..d9056d7 --- /dev/null +++ b/src/tycoon/ingestion/factory.py @@ -0,0 +1,85 @@ +"""SourceFactory — spec-driven config collection for dlt sources. + +All behaviour is driven by SourceSpec fields; there is no +``if source_type == "github"`` branching here. +""" + +from __future__ import annotations + +from typing import Any + +import typer + +from tycoon.ingestion.manifest import SourceSpec +from tycoon.utils.console import console, info + + +class SourceFactory: + def __init__(self, spec: SourceSpec) -> None: + self.spec = spec + + def collect_config( + self, + *, + no_prompt: bool = False, + flags: dict[str, str] | None = None, + ) -> dict[str, Any]: + """Collect config for this source from prompts or flags. + + Interactive mode (no_prompt=False): + - credentials: prompt with hide_input, default "${ENV_VAR}" + - config_fields: prompt with label, default=field.default when set + + Non-interactive mode (no_prompt=True): + - credentials: read from flags or fall back to "${ENV_VAR}" + - config_fields: read from flags; raise SystemExit(1) if required and missing + + Returns a flat config dict: {"access_token": "${GITHUB_TOKEN}", "owner": "dlt-hub", ...} + """ + flags = flags or {} + cfg: dict[str, Any] = {} + + for cred in self.spec.credentials: + default = f"${{{cred.env_var}}}" + if no_prompt: + cfg[cred.key] = flags.get(cred.key, default) + else: + if cred.hint: + console.print(f" [dim]{cred.hint}[/dim]") + value = typer.prompt( + f" {cred.label}", + default=default, + hide_input=cred.secret, + show_default=True, + ) + cfg[cred.key] = value + + for field in self.spec.config_fields: + if no_prompt: + if field.key in flags: + cfg[field.key] = flags[field.key] + elif field.required: + info( + f"--config {field.key}= is required for " + f"[bold]{self.spec.id}[/bold] under --no-prompt." + ) + raise SystemExit(1) + elif field.default is not None: + if field.default != "": + cfg[field.key] = field.default + else: + if field.hint: + console.print(f" [dim]{field.hint}[/dim]") + if field.required: + value = typer.prompt(f" {field.label}") + cfg[field.key] = value + else: + value = typer.prompt( + f" {field.label}", + default=field.default or "", + show_default=bool(field.default), + ) + if value: + cfg[field.key] = value + + return cfg diff --git a/tests/test_factory.py b/tests/test_factory.py new file mode 100644 index 0000000..ebfad9d --- /dev/null +++ b/tests/test_factory.py @@ -0,0 +1,147 @@ +"""Tests for tycoon.ingestion.factory — SourceFactory.collect_config().""" + +from __future__ import annotations + +import pytest + +from tycoon.ingestion.factory import SourceFactory +from tycoon.ingestion.manifest import ConfigField, CredentialField, SourceSpec, load_manifest + + +def _spec_with(**overrides) -> SourceSpec: + defaults = dict(id="test", display_name="Test", category="Test", description="desc") + defaults.update(overrides) + return SourceSpec(**defaults) + + +class TestCollectConfigNonInteractive: + def test_credential_defaults_to_env_var_ref_when_flag_absent(self): + spec = load_manifest()["github"] + factory = SourceFactory(spec) + config = factory.collect_config(no_prompt=True, flags={"owner": "dlt-hub", "repo": "dlt"}) + assert config["access_token"] == "${GITHUB_TOKEN}" + + def test_flags_override_credential_default(self): + spec = load_manifest()["github"] + config = SourceFactory(spec).collect_config( + no_prompt=True, + flags={"access_token": "ghp_abc123", "owner": "acme", "repo": "widgets"}, + ) + assert config["access_token"] == "ghp_abc123" + + def test_required_config_field_present_in_flags(self): + spec = load_manifest()["github"] + config = SourceFactory(spec).collect_config( + no_prompt=True, + flags={"owner": "dlt-hub", "repo": "dlt"}, + ) + assert config["owner"] == "dlt-hub" + assert config["repo"] == "dlt" + + def test_missing_required_config_field_raises_exit(self): + spec = load_manifest()["github"] + with pytest.raises(SystemExit): + SourceFactory(spec).collect_config(no_prompt=True, flags={}) + + def test_missing_one_required_field_raises_exit(self): + spec = load_manifest()["github"] + with pytest.raises(SystemExit): + SourceFactory(spec).collect_config(no_prompt=True, flags={"owner": "acme"}) + + def test_optional_field_absent_keeps_default(self): + spec = load_manifest()["slack"] + config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert "channel_ids" not in config + + def test_optional_field_with_non_empty_default_included(self): + spec = _spec_with( + config_fields=[ + ConfigField(key="base_url", label="Base URL", required=False, default="https://example.com") + ] + ) + config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert config["base_url"] == "https://example.com" + + def test_optional_field_with_empty_default_excluded(self): + spec = _spec_with( + config_fields=[ConfigField(key="ids", label="IDs", required=False, default="")] + ) + config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert "ids" not in config + + def test_source_with_no_fields_returns_empty_dict(self): + spec = load_manifest()["airtable"] + config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert config == {} + + def test_none_flags_treated_as_empty(self): + spec = load_manifest()["airtable"] + config = SourceFactory(spec).collect_config(no_prompt=True, flags=None) + assert config == {} + + def test_rest_api_no_credentials_no_required_fields(self): + spec = load_manifest()["rest_api"] + config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert "access_token" not in config + + def test_multiple_credentials_all_defaulted(self): + spec = _spec_with( + credentials=[ + CredentialField(key="key1", env_var="KEY_ONE", label="Key One"), + CredentialField(key="key2", env_var="KEY_TWO", label="Key Two"), + ] + ) + config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert config["key1"] == "${KEY_ONE}" + assert config["key2"] == "${KEY_TWO}" + + +class TestCollectConfigInteractive: + """Interactive-mode tests use typer.testing.CliRunner to drive prompts.""" + + def _run_factory(self, spec: SourceSpec, input_text: str) -> dict: + import typer + from typer.testing import CliRunner + + _app = typer.Typer() + + result_holder: dict = {} + + @_app.command() + def _cmd() -> None: + result_holder["config"] = SourceFactory(spec).collect_config(no_prompt=False) + + runner = CliRunner() + runner.invoke(_app, [], input=input_text) + return result_holder.get("config", {}) + + def test_interactive_credential_uses_env_var_default(self): + spec = load_manifest()["github"] + config = self._run_factory(spec, "\ndlt-hub\ndlt\n") + assert config.get("access_token") == "${GITHUB_TOKEN}" + + def test_interactive_credential_explicit_value(self): + spec = load_manifest()["github"] + config = self._run_factory(spec, "ghp_token\ndlt-hub\ndlt\n") + assert config.get("access_token") == "ghp_token" + + def test_interactive_required_config_field(self): + spec = load_manifest()["github"] + config = self._run_factory(spec, "\ndlt-hub\ndlt\n") + assert config.get("owner") == "dlt-hub" + assert config.get("repo") == "dlt" + + def test_interactive_optional_field_blank_excluded(self): + spec = load_manifest()["slack"] + config = self._run_factory(spec, "\n\n") + assert "channel_ids" not in config + + def test_interactive_optional_field_filled_included(self): + spec = load_manifest()["slack"] + config = self._run_factory(spec, "\nC01234567\n") + assert config.get("channel_ids") == "C01234567" + + def test_interactive_source_with_no_fields_returns_empty(self): + spec = load_manifest()["airtable"] + config = self._run_factory(spec, "") + assert config == {} From cd3f085ea3a2af0a49787f3240141b0de10f6458 Mon Sep 17 00:00:00 2001 From: Jesufemi-O Date: Sun, 2 Aug 2026 15:29:17 -0400 Subject: [PATCH 2/4] =?UTF-8?q?fix(t3-3):=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20assert=20exit=20code=201,=20test=20optional=20defau?= =?UTF-8?q?lts=20on=20real=20manifest=20entry,=20cover=20default=3DNone=20?= =?UTF-8?q?path,=20use=20explicit=20None-check=20instead=20of=20or-empty?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/tycoon/ingestion/factory.py | 20 ++++---------------- tests/test_factory.py | 20 ++++++++++++++++---- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/tycoon/ingestion/factory.py b/src/tycoon/ingestion/factory.py index d9056d7..dbc146d 100644 --- a/src/tycoon/ingestion/factory.py +++ b/src/tycoon/ingestion/factory.py @@ -24,18 +24,7 @@ def collect_config( no_prompt: bool = False, flags: dict[str, str] | None = None, ) -> dict[str, Any]: - """Collect config for this source from prompts or flags. - - Interactive mode (no_prompt=False): - - credentials: prompt with hide_input, default "${ENV_VAR}" - - config_fields: prompt with label, default=field.default when set - - Non-interactive mode (no_prompt=True): - - credentials: read from flags or fall back to "${ENV_VAR}" - - config_fields: read from flags; raise SystemExit(1) if required and missing - - Returns a flat config dict: {"access_token": "${GITHUB_TOKEN}", "owner": "dlt-hub", ...} - """ + """Produce a flat config dict from spec-driven prompts or flags.""" flags = flags or {} cfg: dict[str, Any] = {} @@ -64,9 +53,8 @@ def collect_config( f"[bold]{self.spec.id}[/bold] under --no-prompt." ) raise SystemExit(1) - elif field.default is not None: - if field.default != "": - cfg[field.key] = field.default + elif field.default is not None and field.default != "": + cfg[field.key] = field.default else: if field.hint: console.print(f" [dim]{field.hint}[/dim]") @@ -76,7 +64,7 @@ def collect_config( else: value = typer.prompt( f" {field.label}", - default=field.default or "", + default=field.default if field.default is not None else "", show_default=bool(field.default), ) if value: diff --git a/tests/test_factory.py b/tests/test_factory.py index ebfad9d..30354b6 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -40,13 +40,15 @@ def test_required_config_field_present_in_flags(self): def test_missing_required_config_field_raises_exit(self): spec = load_manifest()["github"] - with pytest.raises(SystemExit): + with pytest.raises(SystemExit) as exc_info: SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert exc_info.value.code == 1 def test_missing_one_required_field_raises_exit(self): spec = load_manifest()["github"] - with pytest.raises(SystemExit): + with pytest.raises(SystemExit) as exc_info: SourceFactory(spec).collect_config(no_prompt=True, flags={"owner": "acme"}) + assert exc_info.value.code == 1 def test_optional_field_absent_keeps_default(self): spec = load_manifest()["slack"] @@ -79,10 +81,19 @@ def test_none_flags_treated_as_empty(self): config = SourceFactory(spec).collect_config(no_prompt=True, flags=None) assert config == {} - def test_rest_api_no_credentials_no_required_fields(self): + def test_rest_api_optional_defaults_included(self): spec = load_manifest()["rest_api"] config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) assert "access_token" not in config + assert config["base_url"] == "https://pokeapi.co/api/v2/" + assert config["resources"] == "pokemon,berry,type" + + def test_optional_field_with_none_default_excluded(self): + spec = _spec_with( + config_fields=[ConfigField(key="ids", label="IDs", required=False, default=None)] + ) + config = SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert "ids" not in config def test_multiple_credentials_all_defaulted(self): spec = _spec_with( @@ -112,7 +123,8 @@ def _cmd() -> None: result_holder["config"] = SourceFactory(spec).collect_config(no_prompt=False) runner = CliRunner() - runner.invoke(_app, [], input=input_text) + result = runner.invoke(_app, [], input=input_text) + assert result.exit_code == 0, f"CLI crashed: {result.exception}" return result_holder.get("config", {}) def test_interactive_credential_uses_env_var_default(self): From e93d27841749034ffef665907da949f3fdf8a930 Mon Sep 17 00:00:00 2001 From: Jesufemi-O Date: Sun, 2 Aug 2026 15:57:15 -0400 Subject: [PATCH 3/4] fix(t3-3): UX improvements and error handling to factory.collect_config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - error() instead of info() for missing required field — writes to stderr with red prefix, correct for fatal failures - Collect all missing required fields before exiting so --no-prompt users see every missing flag in one run instead of one per re-run - raise typer.Exit(1) instead of SystemExit(1) — consistent with the rest of the Typer command layer; update tests accordingly - Section headers (Credentials / Configuration) in interactive mode when both blocks are non-empty, matching _prompt_catalog_config style - Append '(optional)' to non-required field prompts — system-level cue rather than relying on label wording alone - Print explanatory message when source has no fields (interactive path) so users don't see an abrupt silence after source selection - Single-line module docstring (4-line block condensed) --- src/tycoon/ingestion/factory.py | 42 ++++++++++++++++++++------------- tests/test_factory.py | 15 ++++++++---- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/tycoon/ingestion/factory.py b/src/tycoon/ingestion/factory.py index dbc146d..9dbc325 100644 --- a/src/tycoon/ingestion/factory.py +++ b/src/tycoon/ingestion/factory.py @@ -1,8 +1,4 @@ -"""SourceFactory — spec-driven config collection for dlt sources. - -All behaviour is driven by SourceSpec fields; there is no -``if source_type == "github"`` branching here. -""" +"""SourceFactory — spec-driven config collection for dlt sources.""" from __future__ import annotations @@ -11,7 +7,7 @@ import typer from tycoon.ingestion.manifest import SourceSpec -from tycoon.utils.console import console, info +from tycoon.utils.console import console, error, info class SourceFactory: @@ -28,6 +24,11 @@ def collect_config( flags = flags or {} cfg: dict[str, Any] = {} + if not self.spec.credentials and not self.spec.config_fields and not no_prompt: + console.print(" [dim]No fields to configure — setup is handled by dlt init.[/dim]") + + if self.spec.credentials and not no_prompt: + console.print("[bold]Credentials[/bold]") for cred in self.spec.credentials: default = f"${{{cred.env_var}}}" if no_prompt: @@ -43,27 +44,36 @@ def collect_config( ) cfg[cred.key] = value - for field in self.spec.config_fields: - if no_prompt: + if self.spec.config_fields and self.spec.credentials and not no_prompt: + console.print("[bold]Configuration[/bold]") + + if no_prompt: + missing: list[str] = [] + for field in self.spec.config_fields: if field.key in flags: cfg[field.key] = flags[field.key] elif field.required: - info( - f"--config {field.key}= is required for " - f"[bold]{self.spec.id}[/bold] under --no-prompt." - ) - raise SystemExit(1) + missing.append(field.key) elif field.default is not None and field.default != "": cfg[field.key] = field.default - else: + if missing: + for key in missing: + error( + f"--config {key}= is required for " + f"[bold]{self.spec.id}[/bold] under --no-prompt." + ) + raise typer.Exit(1) + else: + for field in self.spec.config_fields: if field.hint: console.print(f" [dim]{field.hint}[/dim]") + label = f" {field.label}" + ("" if field.required else " (optional)") if field.required: - value = typer.prompt(f" {field.label}") + value = typer.prompt(label) cfg[field.key] = value else: value = typer.prompt( - f" {field.label}", + label, default=field.default if field.default is not None else "", show_default=bool(field.default), ) diff --git a/tests/test_factory.py b/tests/test_factory.py index 30354b6..3bdbb2b 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -3,6 +3,7 @@ from __future__ import annotations import pytest +import typer from tycoon.ingestion.factory import SourceFactory from tycoon.ingestion.manifest import ConfigField, CredentialField, SourceSpec, load_manifest @@ -40,15 +41,21 @@ def test_required_config_field_present_in_flags(self): def test_missing_required_config_field_raises_exit(self): spec = load_manifest()["github"] - with pytest.raises(SystemExit) as exc_info: + with pytest.raises(typer.Exit) as exc_info: SourceFactory(spec).collect_config(no_prompt=True, flags={}) - assert exc_info.value.code == 1 + assert exc_info.value.exit_code == 1 def test_missing_one_required_field_raises_exit(self): spec = load_manifest()["github"] - with pytest.raises(SystemExit) as exc_info: + with pytest.raises(typer.Exit) as exc_info: SourceFactory(spec).collect_config(no_prompt=True, flags={"owner": "acme"}) - assert exc_info.value.code == 1 + assert exc_info.value.exit_code == 1 + + def test_all_missing_required_fields_reported_before_exit(self): + spec = load_manifest()["github"] + with pytest.raises(typer.Exit) as exc_info: + SourceFactory(spec).collect_config(no_prompt=True, flags={}) + assert exc_info.value.exit_code == 1 def test_optional_field_absent_keeps_default(self): spec = load_manifest()["slack"] From 38da53c485dae800f198fa011a30fec92141289b Mon Sep 17 00:00:00 2001 From: Jesufemi-O Date: Sun, 2 Aug 2026 16:08:23 -0400 Subject: [PATCH 4/4] fix(t3-3): update _spec_with helper for provider/backend required fields --- tests/test_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_factory.py b/tests/test_factory.py index 3bdbb2b..a9ed519 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -10,7 +10,7 @@ def _spec_with(**overrides) -> SourceSpec: - defaults = dict(id="test", display_name="Test", category="Test", description="desc") + defaults = dict(id="test", provider="test", backend={}, display_name="Test", category="Test", description="desc") defaults.update(overrides) return SourceSpec(**defaults)