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/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..715c9be 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,46 @@ 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 + # 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: + overrides["create_database_if_missing"] = flag.lower() in {"1", "true", "yes"} + retries = os.environ.get("HOTDATA_MAX_RETRIES") + if retries: + overrides["max_retries"] = _parse_max_retries(retries) + backoff = os.environ.get("HOTDATA_RETRY_BACKOFF_SECONDS") + if backoff: + overrides["retry_backoff_seconds"] = _parse_backoff(backoff) + return overrides + + @dataclass(frozen=True) class HotdataDestinationConfig: api_key: str @@ -34,27 +75,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..86feeb2 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,68 @@ 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 + + +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