From 610a7add11a43d3545eed51dbb54f802fea9046f Mon Sep 17 00:00:00 2001 From: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:22:47 -0700 Subject: [PATCH 1/2] fix: wire the documented HOTDATA_* env vars into the destination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Configuration table documents plain HOTDATA_* names, but only HOTDATA_API_KEY was bridged; the rest were read solely by the diagnostic CLI's from_env(), so HOTDATA_DATABASE_ID et al. silently did nothing in a pipeline — including the pin-the-database advice the first-run log message itself gives. The factory now applies plain_env_overrides() (shared with the CLI) for each keyword not passed explicitly, with the same precedence the HOTDATA_API_KEY bridge has always had: explicit param > plain HOTDATA_* > DESTINATION__HOTDATA__* / config.toml > default. The diagnostic CLI's retry defaults are aligned with the destination's (8 / 1.5s, were 5 / 1.0s). Fixes #95 --- CHANGELOG.md | 11 +++++ README.md | 2 +- src/hotdata_dlt_destination/config.py | 62 ++++++++++++++++++-------- src/hotdata_dlt_destination/factory.py | 25 +++++++++++ tests/test_factory.py | 62 ++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92480fb..be541a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,17 @@ Changelog style — terse "bullet + brief why": **Breaking:**. See the released entries below for the target density. --> +### Fixed + +- The documented `HOTDATA_*` env vars now reach the destination (previously only + `HOTDATA_API_KEY` did); explicit params still win, and the plain names take + precedence over `DESTINATION__HOTDATA__*` / `config.toml` (#95). + +### Changed + +- Diagnostic CLI retry defaults aligned with the destination's: `max_retries` 8 + (was 5), `retry_backoff_seconds` 1.5 (was 1.0). + ## [0.16.0] - 2026-09-01 ### Added diff --git a/README.md b/README.md index 5b3dd19..8df34a6 100644 --- a/README.md +++ b/README.md @@ -371,7 +371,7 @@ Notes: | `retry_backoff_seconds` | `HOTDATA_RETRY_BACKOFF_SECONDS` | `1.5` | Initial wait between retries (grows linearly with each attempt) | | `max_state_files` | `DESTINATION__HOTDATA__MAX_STATE_FILES` | `100` | How many `_dlt_pipeline_state` rows to keep per pipeline. Only the newest is ever read; older rows are trimmed. Set `0` to keep every row | -Pass these as keyword arguments to `hotdata(...)`. The `api_key` is the exception — being a secret, it's read from `HOTDATA_API_KEY` (or supplied via `credentials=`, e.g. `hotdata(credentials={"api_key": "..."}, ...)`); `workspace_id` has no environment variable. `hotdata` also accepts: +Pass these as keyword arguments to `hotdata(...)`. The `api_key` is the exception — being a secret, it's read from `HOTDATA_API_KEY` (or supplied via `credentials=`, e.g. `hotdata(credentials={"api_key": "..."}, ...)`); `workspace_id` has no environment variable. When a setting is given more than one way, explicit `hotdata(...)` keywords win, then the plain `HOTDATA_*` variables, then dlt's `DESTINATION__HOTDATA__*` / `config.toml` forms. `hotdata` also accepts: - `max_table_nesting` (default `1000`) — maximum nested/child-table depth. - `loader_parallelism_strategy` (default `sequential`) — instant-database loads lock at the catalog level, so different tables in the same database can't load concurrently. Override only if you know your loads won't contend for the same database. diff --git a/src/hotdata_dlt_destination/config.py b/src/hotdata_dlt_destination/config.py index badfd9b..4175c01 100644 --- a/src/hotdata_dlt_destination/config.py +++ b/src/hotdata_dlt_destination/config.py @@ -2,6 +2,7 @@ import os from dataclasses import dataclass +from typing import Any def _parse_max_retries(value: str) -> int: @@ -24,6 +25,42 @@ def _parse_backoff(value: str) -> float: return n +def plain_env_overrides() -> dict[str, Any]: + """Settings read from the plain ``HOTDATA_*`` environment variables. + + This is the documented environment contract, shared by the destination + factory and the diagnostic CLI. Only variables that are set — and, for the + string-valued ones, non-empty — appear in the result, so an absent key + means "not configured this way" and the caller's default stands. + """ + overrides: dict[str, Any] = {} + for key, name in ( + ("database_id", "HOTDATA_DATABASE_ID"), + ("database_name", "HOTDATA_DATABASE"), + ("schema", "HOTDATA_SCHEMA"), + ("write_disposition", "HOTDATA_WRITE_DISPOSITION"), + ("api_base_url", "HOTDATA_API_BASE_URL"), + ): + value = os.environ.get(name) + if value: + overrides[key] = value + declared = os.environ.get("HOTDATA_DECLARED_TABLES") + if declared is not None: + tables = [table.strip() for table in declared.split(",") if table.strip()] + if tables: + overrides["declared_tables"] = tables + flag = os.environ.get("HOTDATA_CREATE_DATABASE_IF_MISSING") + if flag is not None: + overrides["create_database_if_missing"] = flag.lower() in {"1", "true", "yes"} + retries = os.environ.get("HOTDATA_MAX_RETRIES") + if retries is not None: + overrides["max_retries"] = _parse_max_retries(retries) + backoff = os.environ.get("HOTDATA_RETRY_BACKOFF_SECONDS") + if backoff is not None: + overrides["retry_backoff_seconds"] = _parse_backoff(backoff) + return overrides + + @dataclass(frozen=True) class HotdataDestinationConfig: api_key: str @@ -34,27 +71,16 @@ class HotdataDestinationConfig: write_disposition: str = "append" create_database_if_missing: bool = True declared_tables: tuple[str, ...] = () - max_retries: int = 5 - retry_backoff_seconds: float = 1.0 + max_retries: int = 8 + retry_backoff_seconds: float = 1.5 @classmethod def from_env(cls) -> HotdataDestinationConfig: - declared = os.environ.get("HOTDATA_DECLARED_TABLES", "") - declared_tables = tuple(table.strip() for table in declared.split(",") if table.strip()) + overrides = plain_env_overrides() + if "declared_tables" in overrides: + overrides["declared_tables"] = tuple(overrides["declared_tables"]) return cls( api_key=os.environ["HOTDATA_API_KEY"], - database_name=os.environ.get("HOTDATA_DATABASE", "dlt"), - database_id=os.environ.get("HOTDATA_DATABASE_ID") or None, - api_base_url=os.environ.get("HOTDATA_API_BASE_URL", "https://api.hotdata.dev"), - schema=os.environ.get("HOTDATA_SCHEMA", "public"), - write_disposition=os.environ.get("HOTDATA_WRITE_DISPOSITION", "append"), - create_database_if_missing=os.environ.get( - "HOTDATA_CREATE_DATABASE_IF_MISSING", "true" - ).lower() - in {"1", "true", "yes"}, - declared_tables=declared_tables, - max_retries=_parse_max_retries(os.environ.get("HOTDATA_MAX_RETRIES", "5")), - retry_backoff_seconds=_parse_backoff( - os.environ.get("HOTDATA_RETRY_BACKOFF_SECONDS", "1.0") - ), + database_name=overrides.pop("database_name", "dlt"), + **overrides, ) diff --git a/src/hotdata_dlt_destination/factory.py b/src/hotdata_dlt_destination/factory.py index 23cacb3..c6f79e5 100644 --- a/src/hotdata_dlt_destination/factory.py +++ b/src/hotdata_dlt_destination/factory.py @@ -12,6 +12,7 @@ from dlt.common.normalizers.naming import NamingConvention from dlt.common.wei import EVM_DECIMAL_PRECISION +from hotdata_dlt_destination.config import plain_env_overrides from hotdata_dlt_destination.configuration import ( HotdataClientConfiguration, HotdataCredentials, @@ -124,6 +125,30 @@ def __init__( if credentials is None and os.environ.get("HOTDATA_API_KEY") is not None: credentials = HotdataCredentials(api_key=os.environ["HOTDATA_API_KEY"]) + # The plain HOTDATA_* names are the documented environment contract, + # shared with the diagnostic CLI. Like the HOTDATA_API_KEY bridge above, + # each applies only when the keyword is not passed, and takes precedence + # over dlt's DESTINATION__HOTDATA__* / config.toml forms. + env = plain_env_overrides() + if database_id is None: + database_id = env.get("database_id") + if database_name is None: + database_name = env.get("database_name") + if schema is None: + schema = env.get("schema") + if write_disposition is None: + write_disposition = env.get("write_disposition") + if declared_tables is None: + declared_tables = env.get("declared_tables") + if create_database_if_missing is None: + create_database_if_missing = env.get("create_database_if_missing") + if api_base_url is None: + api_base_url = env.get("api_base_url") + if max_retries is None: + max_retries = env.get("max_retries") + if retry_backoff_seconds is None: + retry_backoff_seconds = env.get("retry_backoff_seconds") + super().__init__( credentials=credentials, workspace_id=workspace_id, diff --git a/tests/test_factory.py b/tests/test_factory.py index fe04288..7495ca8 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -5,8 +5,18 @@ _ENV_KEYS = ( "HOTDATA_API_KEY", + "HOTDATA_DATABASE_ID", + "HOTDATA_DATABASE", + "HOTDATA_SCHEMA", + "HOTDATA_WRITE_DISPOSITION", + "HOTDATA_DECLARED_TABLES", + "HOTDATA_CREATE_DATABASE_IF_MISSING", + "HOTDATA_API_BASE_URL", + "HOTDATA_MAX_RETRIES", + "HOTDATA_RETRY_BACKOFF_SECONDS", "DESTINATION__HOTDATA__CREDENTIALS__API_KEY", "DESTINATION__HOTDATA__WORKSPACE_ID", + "DESTINATION__HOTDATA__SCHEMA", ) @@ -94,3 +104,55 @@ def test_legacy_workspace_in_credentials_dict_is_hoisted_without_mutating(clean_ cfg = _resolve(dest) assert cfg.credentials.api_key == "k" assert cfg.workspace_id == "ws_dict" + + +def test_plain_env_vars_reach_the_destination(clean_env) -> None: + # The documented HOTDATA_* contract (issue #95): these must land in the + # resolved config, not just in the diagnostic CLI's from_env(). + clean_env.setenv("HOTDATA_API_KEY", "sk_env") + clean_env.setenv("HOTDATA_DATABASE_ID", "db_env") + clean_env.setenv("HOTDATA_SCHEMA", "staging") + clean_env.setenv("HOTDATA_MAX_RETRIES", "3") + clean_env.setenv("HOTDATA_RETRY_BACKOFF_SECONDS", "0.5") + clean_env.setenv("HOTDATA_DECLARED_TABLES", "orders, customers") + clean_env.setenv("HOTDATA_CREATE_DATABASE_IF_MISSING", "false") + cfg = _resolve(hotdata(workspace_id="ws")) + assert cfg.database_id == "db_env" + assert cfg.schema == "staging" + assert cfg.max_retries == 3 + assert cfg.retry_backoff_seconds == 0.5 + assert cfg.declared_tables == ["orders", "customers"] + assert cfg.create_database_if_missing is False + + +def test_explicit_param_beats_plain_env(clean_env) -> None: + clean_env.setenv("HOTDATA_API_KEY", "sk_env") + clean_env.setenv("HOTDATA_SCHEMA", "from_env") + cfg = _resolve(hotdata(workspace_id="ws", schema="from_param")) + assert cfg.schema == "from_param" + + +def test_plain_env_beats_dlt_style_env(clean_env) -> None: + # Same precedence the HOTDATA_API_KEY bridge has always had: the plain name + # is bound at construction, so it outranks the provider-resolved form. + clean_env.setenv("HOTDATA_API_KEY", "sk_env") + clean_env.setenv("HOTDATA_SCHEMA", "plain") + clean_env.setenv("DESTINATION__HOTDATA__SCHEMA", "dlt_style") + cfg = _resolve(hotdata(workspace_id="ws")) + assert cfg.schema == "plain" + + +def test_dlt_style_env_still_works_without_plain_var(clean_env) -> None: + clean_env.setenv("HOTDATA_API_KEY", "sk_env") + clean_env.setenv("DESTINATION__HOTDATA__SCHEMA", "dlt_style") + cfg = _resolve(hotdata(workspace_id="ws")) + assert cfg.schema == "dlt_style" + + +def test_unset_plain_env_leaves_defaults(clean_env) -> None: + clean_env.setenv("HOTDATA_API_KEY", "sk_env") + cfg = _resolve(hotdata(workspace_id="ws")) + assert cfg.schema == "public" + assert cfg.database_name == "dlt" + assert cfg.max_retries == 8 + assert cfg.retry_backoff_seconds == 1.5 From 9b0f728a61a0ddb19d826ae2d1454c459dea1484 Mon Sep 17 00:00:00 2001 From: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:27:18 -0700 Subject: [PATCH 2/2] fix review: blank plain env vars are unset, not errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A set-but-blank HOTDATA_MAX_RETRIES or HOTDATA_RETRY_BACKOFF_SECONDS raised out of the hotdata(...) constructor, and a blank HOTDATA_CREATE_DATABASE_IF_MISSING silently flipped to False — .env.example ships blank values as the unset convention. Truthiness checks now match the string settings; test added. .env.example retry values aligned with the destination defaults (8 / 1.5). --- .env.example | 4 ++-- src/hotdata_dlt_destination/config.py | 10 +++++++--- tests/test_factory.py | 13 +++++++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index 8d24e23..449a883 100644 --- a/.env.example +++ b/.env.example @@ -9,8 +9,8 @@ HOTDATA_WRITE_DISPOSITION=append HOTDATA_DECLARED_TABLES=customers,orders HOTDATA_CREATE_DATABASE_IF_MISSING=true HOTDATA_API_BASE_URL=https://api.hotdata.dev -HOTDATA_MAX_RETRIES=5 -HOTDATA_RETRY_BACKOFF_SECONDS=1.0 +HOTDATA_MAX_RETRIES=8 +HOTDATA_RETRY_BACKOFF_SECONDS=1.5 # Linear pipeline (optional) LINEAR_API_KEY= diff --git a/src/hotdata_dlt_destination/config.py b/src/hotdata_dlt_destination/config.py index 4175c01..715c9be 100644 --- a/src/hotdata_dlt_destination/config.py +++ b/src/hotdata_dlt_destination/config.py @@ -49,14 +49,18 @@ def plain_env_overrides() -> dict[str, Any]: tables = [table.strip() for table in declared.split(",") if table.strip()] if tables: overrides["declared_tables"] = tables + # Truthiness, not `is not None`: a set-but-blank variable means unset here, + # the same convention the string settings above follow and .env.example ships + # (`HOTDATA_DATABASE_ID=`). An `is not None` check would raise out of the + # hotdata(...) constructor — or silently flip the create flag — on a blank. flag = os.environ.get("HOTDATA_CREATE_DATABASE_IF_MISSING") - if flag is not None: + if flag: overrides["create_database_if_missing"] = flag.lower() in {"1", "true", "yes"} retries = os.environ.get("HOTDATA_MAX_RETRIES") - if retries is not None: + if retries: overrides["max_retries"] = _parse_max_retries(retries) backoff = os.environ.get("HOTDATA_RETRY_BACKOFF_SECONDS") - if backoff is not None: + if backoff: overrides["retry_backoff_seconds"] = _parse_backoff(backoff) return overrides diff --git a/tests/test_factory.py b/tests/test_factory.py index 7495ca8..86feeb2 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -156,3 +156,16 @@ def test_unset_plain_env_leaves_defaults(clean_env) -> None: assert cfg.database_name == "dlt" assert cfg.max_retries == 8 assert cfg.retry_backoff_seconds == 1.5 + + +def test_blank_plain_env_vars_are_treated_as_unset(clean_env) -> None: + # .env.example ships blank values (HOTDATA_DATABASE_ID=); a blank must mean + # "unset", not a parse error out of the constructor or a flipped flag. + clean_env.setenv("HOTDATA_API_KEY", "sk_env") + clean_env.setenv("HOTDATA_MAX_RETRIES", "") + clean_env.setenv("HOTDATA_RETRY_BACKOFF_SECONDS", "") + clean_env.setenv("HOTDATA_CREATE_DATABASE_IF_MISSING", "") + cfg = _resolve(hotdata(workspace_id="ws")) + assert cfg.max_retries == 8 + assert cfg.retry_backoff_seconds == 1.5 + assert cfg.create_database_if_missing is True