From 0d55474ee727c18149b79f9c6210a502fe6f2b0e Mon Sep 17 00:00:00 2001 From: oskaresparza Date: Fri, 2 Oct 2026 12:25:26 +0200 Subject: [PATCH 1/4] Reject an invalid target_name in push_to_dds before anything else, ref #306883 - target_name must be a str file name: an SdiMetadata passed there raises with a hint to use metadata=; empty names, paths and characters no file system accepts (< > : " | ? * and control characters) raise ValueError. - %metadata push_to_dds validates its arguments before looking up any DDS or Dremio settings, so the error names the bad argument. - metadata_path accepts the record in place of its UUID; a non-str uuid raises TypeError. Co-Authored-By: Claude Opus 5.5 Redmine-Hook: v1 --- src/eea_datalakehouse/sdi/controller.py | 30 +++++++++++++++++-- src/eea_datalakehouse/sdi/session.py | 14 +++++++-- tests/sdi/test_controller.py | 19 +++++++++++- tests/sdi/test_session.py | 40 +++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/eea_datalakehouse/sdi/controller.py b/src/eea_datalakehouse/sdi/controller.py index f396d31..4fccc5d 100644 --- a/src/eea_datalakehouse/sdi/controller.py +++ b/src/eea_datalakehouse/sdi/controller.py @@ -345,7 +345,7 @@ def to_catalog_path(path: str) -> str: def metadata_path( dds_path: str, - uuid: str, + uuid: str | SdiMetadata, *, folder: str = DEFAULT_FOLDER, target_name: str | None = None, @@ -355,8 +355,10 @@ def metadata_path( ``dds_path`` may be in catalog format (``a.b.c``) or storage format (``a/b/c``); see :func:`to_storage_path`. ``target_name`` is the file name, ``{uuid}.xml`` by default; ``.xml`` is added if it has no such - extension. + extension. ``uuid`` may also be the :class:`SdiMetadata` itself. """ + if isinstance(uuid, SdiMetadata): + uuid = uuid.uuid parent = to_storage_path(dds_path).strip("/") sub = folder.strip().strip("/").lower() if not parent: @@ -368,19 +370,41 @@ def metadata_path( return f"{parent}/{sub}/{name}" +_INVALID_NAME_CHARS = set('<>:"|?*') + + def _require_target_name(target_name: str) -> str: - """``target_name`` as a single file name ending in ``.xml``.""" + """``target_name`` as a valid single file name ending in ``.xml``. + + Raises ``TypeError`` unless it is a ``str`` (an :class:`SdiMetadata` + passed here by mistake gets a hint to use ``metadata=``), and + ``ValueError`` if it is empty, a path (``/``, ``\\``, ``.``, ``..``), or + has a character no file system accepts in a name (``< > : " | ? *`` or a + control character). + """ + if not isinstance(target_name, str): + raise TypeError( + f"target_name must be a file name (str), not {type(target_name).__name__}" + + (" — pass the record as metadata=..." if isinstance(target_name, SdiMetadata) else "") + ) name = target_name.strip() if not name: raise ValueError("target_name must not be empty") if "/" in name or "\\" in name or name in (".", ".."): raise ValueError(f"target_name must be a file name, not a path: {target_name!r}") + bad = sorted({c for c in name if c in _INVALID_NAME_CHARS or ord(c) < 32 or ord(c) == 127}) + if bad: + raise ValueError( + f"target_name {target_name!r} is not a valid file name: it contains {bad!r}" + ) if not name.lower().endswith(".xml"): name += ".xml" return name def _require_uuid(uuid: str) -> str: + if not isinstance(uuid, str): + raise TypeError(f"uuid must be a str, not {type(uuid).__name__}") uuid = uuid.strip() if not uuid: raise ValueError("uuid must not be empty") diff --git a/src/eea_datalakehouse/sdi/session.py b/src/eea_datalakehouse/sdi/session.py index c617da7..b97c936 100644 --- a/src/eea_datalakehouse/sdi/session.py +++ b/src/eea_datalakehouse/sdi/session.py @@ -58,7 +58,7 @@ MissingCredentialsError, ) from .catalogue import SdiCatalogue -from .controller import DEFAULT_FOLDER, PushResult, SdiController, SdiMetadata +from .controller import DEFAULT_FOLDER, PushResult, SdiController, SdiMetadata, metadata_path from .errors import SdiError # What `%catalog` reads (see notebook/magics.py's _build_catalog_session). @@ -70,7 +70,14 @@ _F = TypeVar("_F", bound=Callable[..., Any]) # Failures a notebook user can act on; anything else is a bug and keeps its traceback. -_EXPECTED = (SdiError, DocumentsApiError, MissingCredentialsError, ValueError, httpx.HTTPError) +_EXPECTED = ( + SdiError, + DocumentsApiError, + MissingCredentialsError, + ValueError, + TypeError, + httpx.HTTPError, +) class SdiSessionError(RuntimeError): @@ -244,6 +251,9 @@ def push_to_dds( metadata = metadata or self._last() if metadata is None: raise MetadataSessionError("nothing to push yet — run %sdi get_xml(uuid) first") + # Arguments first: a bad target_name / dds_path / folder is reported as + # such, before any DDS or Dremio settings are looked up. + metadata_path(dds_path, metadata.uuid, folder=folder, target_name=target_name) return self._push_controller(with_catalog=check_catalog).push_to_dds( metadata, dds_path, diff --git a/tests/sdi/test_controller.py b/tests/sdi/test_controller.py index 4037af6..9ea7777 100644 --- a/tests/sdi/test_controller.py +++ b/tests/sdi/test_controller.py @@ -138,13 +138,30 @@ def test_to_catalog_path(path: str, expected: str) -> None: assert to_storage_path(expected) == to_storage_path(path) # round trip +def test_metadata_path_accepts_the_record_for_uuid(sdi: SdiController) -> None: + assert metadata_path("water.bathing_water", extracted(sdi)) == TARGET + + +def test_metadata_path_rejects_the_record_as_target_name(sdi: SdiController) -> None: + record = extracted(sdi) + with pytest.raises(TypeError, match="not SdiMetadata — pass the record as metadata="): + metadata_path("water", UUID, target_name=record) # type: ignore[arg-type] + + +def test_metadata_path_rejects_non_str_uuid() -> None: + with pytest.raises(TypeError, match="uuid must be a str"): + metadata_path("water", 42) # type: ignore[arg-type] + + def test_metadata_path_target_name() -> None: assert metadata_path("water", UUID, target_name="bwd_2025.xml") == "water/metadata/bwd_2025.xml" assert metadata_path("water", UUID, target_name=" bwd_2025 ") == "water/metadata/bwd_2025.xml" assert metadata_path("water", UUID, target_name="A.XML") == "water/metadata/A.XML" -@pytest.mark.parametrize("bad", ["", " ", "a/b.xml", "..", "a\\b.xml"]) +@pytest.mark.parametrize( + "bad", ["", " ", "a/b.xml", "..", "a\\b.xml", "a:b.xml", "what?.xml", 'a"b', "a\tb", "x*"] +) def test_metadata_path_rejects_bad_target_name(bad: str) -> None: with pytest.raises(ValueError, match="target_name"): metadata_path("water", UUID, target_name=bad) diff --git a/tests/sdi/test_session.py b/tests/sdi/test_session.py index 2f5b2a4..c88f0c2 100644 --- a/tests/sdi/test_session.py +++ b/tests/sdi/test_session.py @@ -282,3 +282,43 @@ def test_push_target_name_renames_the_file() -> None: assert result.dds_path == renamed assert put.called + + +@respx.mock +def test_push_refuses_the_record_as_target_name() -> None: + respx.get(record_url()).mock(return_value=httpx.Response(200, content=iso_xml())) + dds = respx.route(host="dds.example.test") + sdi = SdiSession(env=ENV) + record = sdi.get_xml(UUID) + + with pytest.raises( + MetadataSessionError, match=r"not SdiMetadata — pass the record as metadata=\.\.\." + ): + _metadata_session(sdi, env={**ENV, **IDENTITY}).push_to_dds( + "water", record, check_catalog=False # type: ignore[arg-type] + ) + assert not dds.called + + +@respx.mock +def test_push_wrong_target_name_type_is_a_session_error() -> None: + respx.get(record_url()).mock(return_value=httpx.Response(200, content=iso_xml())) + sdi = SdiSession(env=ENV) + sdi.get_xml(UUID) + + with pytest.raises(MetadataSessionError, match="target_name must be a file name"): + _metadata_session(sdi, env={**ENV, **IDENTITY}).push_to_dds( + "water", 42, check_catalog=False # type: ignore[arg-type] + ) + + +@respx.mock +@pytest.mark.parametrize("bad", ["bad:name.xml", "a/b.xml", ""]) +def test_bad_target_name_is_reported_before_any_settings(bad: str) -> None: + respx.get(record_url()).mock(return_value=httpx.Response(200, content=iso_xml())) + sdi = SdiSession(env=ENV) + sdi.get_xml(UUID) + no_settings = _metadata_session(sdi, env={}) # no DDS URL, no Dremio identity + + with pytest.raises(MetadataSessionError, match="target_name"): + no_settings.push_to_dds("water", bad) From 36e14dc1076fa8501cf8a29ad9b60203f8430e6b Mon Sep 17 00:00:00 2001 From: oskaresparza Date: Fri, 2 Oct 2026 12:37:11 +0200 Subject: [PATCH 2/4] Strict argument checks and clearer call errors for push_to_dds, ref #306883 - push_to_dds (session and controller) checks every argument's type in parameter order, positional or named: str/bool parameters take nothing else, and the message names the argument and its position. - A call that doesn't fit the signature (unknown keyword, an argument given twice, too many arguments) reports why plus the correct form of the call. - A magic line that isn't valid Python (e.g. an unnamed argument after a name=value one) names the offending argument, suggests parameters its value fits, and shows the usage, for every magic. Co-Authored-By: Claude Opus 5.5 Redmine-Hook: v1 --- src/eea_datalakehouse/notebook/magics.py | 106 +++++++++++++++++++++++ src/eea_datalakehouse/sdi/controller.py | 43 +++++++++ src/eea_datalakehouse/sdi/session.py | 49 ++++++++++- tests/notebook/test_magics.py | 68 +++++++++++++++ tests/sdi/test_controller.py | 10 +++ tests/sdi/test_session.py | 42 ++++++++- 6 files changed, 313 insertions(+), 5 deletions(-) diff --git a/src/eea_datalakehouse/notebook/magics.py b/src/eea_datalakehouse/notebook/magics.py index 2587643..e24dae3 100644 --- a/src/eea_datalakehouse/notebook/magics.py +++ b/src/eea_datalakehouse/notebook/magics.py @@ -86,8 +86,10 @@ from __future__ import annotations +import ast import inspect import os +import re from html import escape as _escape from typing import Any @@ -412,6 +414,106 @@ def _print_help_table( display(HTML(_help_table_html(rows))) +_CALL = re.compile(r"\s*([A-Za-z_]\w*)\s*\((.*)\)\s*", re.DOTALL) +_CALLEE = re.compile(r"\s*([A-Za-z_]\w*)\s*\(") +_KEYWORD_ARG = re.compile(r"\s*([A-Za-z_]\w*)\s*=(?!=)") + + +def _split_args(text: str) -> list[str] | None: + """The top-level, comma-separated arguments of a call, as written. + + Commas inside brackets or string literals don't split. ``None`` if the + brackets or quotes don't balance — nothing useful can be said then. + """ + args: list[str] = [] + depth = 0 + quote: str | None = None + start = 0 + i = 0 + while i < len(text): + char = text[i] + if quote: + if char == "\\": + i += 1 + elif char == quote: + quote = None + elif char in "\"'": + quote = char + elif char in "([{": + depth += 1 + elif char in ")]}": + depth -= 1 + if depth < 0: + return None + elif char == "," and depth == 0: + args.append(text[start:i].strip()) + start = i + 1 + i += 1 + if quote or depth: + return None + last = text[start:].strip() + if last: + args.append(last) + return args + + +def _fits(annotation: Any, value_text: str) -> bool: + """Whether the literal ``value_text`` could be a value for ``annotation``. + + Anything that isn't a literal (a variable name, an expression) could be + anything, so it fits every parameter. + """ + try: + value = ast.literal_eval(value_text) + except (ValueError, SyntaxError): + return True + if annotation is inspect.Parameter.empty: + return True + allowed = {part.strip() for part in str(annotation).split("|")} + return type(value).__name__ in allowed or (value is None and "None" in allowed) + + +def _explain_syntax_error(session: Any, line: str, exc: SyntaxError) -> str: + """A magic line that isn't valid Python, explained against the method it calls. + + For the commonest slip — an unnamed argument after a ``name=value`` one — + names the offending argument and suggests the parameters it could be; + always ends with the correct form of the call when the method is known. + """ + callee = _CALLEE.match(line) + method = getattr(type(session), callee.group(1), None) if callee else None + if not callable(method): + return f"invalid syntax: {exc.msg}" + usage = f"usage: {method.__name__}({_plain_params(method)})" + match = _CALL.fullmatch(line) + args = _split_args(match.group(2)) if match else None + if args: + keywords = [_KEYWORD_ARG.match(arg) for arg in args] + first_keyword = next((i for i, kw in enumerate(keywords) if kw), None) + stray = next( + (i for i, kw in enumerate(keywords) if first_keyword is not None + and i > first_keyword and kw is None), + None, + ) + if first_keyword is not None and stray is not None: + named = {kw.group(1) for kw in keywords if kw} + params = [ + p for name, p in inspect.signature(method).parameters.items() if name != "self" + ] + candidates = [ + f"{p.name}={args[stray]}" + for p in params[first_keyword:] + if p.name not in named and _fits(p.annotation, args[stray]) + ] + hint = f" Did you mean {' or '.join(candidates)}?" if candidates else "" + return ( + f"argument {stray + 1} ({args[stray]}) has no name, but comes after " + f"{args[first_keyword]} — once an argument is given by name, every argument " + f"after it needs a name too.{hint}\n{usage}" + ) + return f"invalid syntax: {exc.msg}\n{usage}" + + _DISPATCH_FAILED = object() # Sentinel `_dispatch` returns when it printed a friendly error — distinct # from a legitimate `None` result (e.g. `use(...)` printing its own status @@ -453,6 +555,10 @@ def _dispatch( # the context they just set instead of an empty CommitReport. print(f"context set to {session.get_context()!r}") result = None + except SyntaxError as exc: + # The line itself isn't valid Python, so the call never happened. + print(f"{label} error: {_explain_syntax_error(session, line, exc)}") + return _DISPATCH_FAILED except error_type as exc: print(f"{label} error: {exc}") return _DISPATCH_FAILED diff --git a/src/eea_datalakehouse/sdi/controller.py b/src/eea_datalakehouse/sdi/controller.py index 4fccc5d..6619d62 100644 --- a/src/eea_datalakehouse/sdi/controller.py +++ b/src/eea_datalakehouse/sdi/controller.py @@ -199,6 +199,15 @@ def push_to_dds( The XML is re-checked first, so a hand-built :class:`SdiMetadata` cannot put one record under another's UUID. """ + check_arg_types( + "push_to_dds", + ("metadata", metadata, SdiMetadata, False), + ("dds_path", dds_path, str, False), + ("target_name", target_name, str, True), + ("folder", folder, str, False), + ("force", force, bool, False), + ("check_catalog", check_catalog, bool, False), + ) iso.check(metadata.xml, metadata.uuid) target = metadata_path(dds_path, metadata.uuid, folder=folder, target_name=target_name) if check_catalog and self._catalog is not None: @@ -373,6 +382,40 @@ def metadata_path( _INVALID_NAME_CHARS = set('<>:"|?*') +def check_arg_types( + func: str, *specs: tuple[str, object, type | tuple[type, ...], bool] +) -> None: + """Raise ``TypeError`` unless every argument has exactly its declared type. + + ``specs`` are ``(name, value, expected, optional)`` in the function's + parameter order, so the message can name the position too — the argument + most likely to be wrong is one passed positionally into the wrong slot. + Strict: ``bool`` parameters take only ``True`` / ``False`` (not ``0`` / + ``1``), and a ``str`` parameter takes no other type. ``optional`` allows + ``None``. + """ + for position, (name, value, expected, optional) in enumerate(specs, start=1): + if value is None and optional: + continue + kinds = expected if isinstance(expected, tuple) else (expected,) + # bool is a subclass of int: only a bool parameter may take a bool. + if any( + isinstance(value, kind) and (kind is bool or not isinstance(value, bool)) + for kind in kinds + ): + continue + wanted = " or ".join(kind.__name__ for kind in kinds) + (" or None" if optional else "") + hint = ( + " — pass the record as metadata=..." + if isinstance(value, SdiMetadata) and name != "metadata" + else "" + ) + raise TypeError( + f"{func}: argument {position} ({name}) must be {wanted}, " + f"not {type(value).__name__}{hint}" + ) + + def _require_target_name(target_name: str) -> str: """``target_name`` as a valid single file name ending in ``.xml``. diff --git a/src/eea_datalakehouse/sdi/session.py b/src/eea_datalakehouse/sdi/session.py index b97c936..66103b8 100644 --- a/src/eea_datalakehouse/sdi/session.py +++ b/src/eea_datalakehouse/sdi/session.py @@ -40,6 +40,7 @@ from __future__ import annotations +import inspect import os from collections.abc import Callable, Mapping from functools import wraps @@ -58,7 +59,14 @@ MissingCredentialsError, ) from .catalogue import SdiCatalogue -from .controller import DEFAULT_FOLDER, PushResult, SdiController, SdiMetadata, metadata_path +from .controller import ( + DEFAULT_FOLDER, + PushResult, + SdiController, + SdiMetadata, + check_arg_types, + metadata_path, +) from .errors import SdiError # What `%catalog` reads (see notebook/magics.py's _build_catalog_session). @@ -88,12 +96,35 @@ class MetadataSessionError(RuntimeError): """Any failure of a `MetadataSession` call; the original error is ``__cause__``.""" +def call_usage(method: Callable[..., Any]) -> str: + """``name(a, b=None, ...)`` — how to call ``method``, without ``self`` or type hints.""" + params = [ + name if param.default is inspect.Parameter.empty else f"{name}={param.default!r}" + for name, param in inspect.signature(method).parameters.items() + if name != "self" + ] + return f"{method.__name__}({', '.join(params)})" + + def _friendly(error: type[RuntimeError]) -> Callable[[_F], _F]: - """Re-raise every expected failure of the decorated method as ``error``.""" + """Re-raise every expected failure of the decorated method as ``error``. + + A call that doesn't fit the method's signature (an unknown keyword, an + argument given twice, too many arguments) is reported the same way, with + the correct form of the call. + """ def decorate(method: _F) -> _F: + signature = inspect.signature(method) + @wraps(method) def wrapper(*args: Any, **kwargs: Any) -> Any: + try: + signature.bind(*args, **kwargs) + except TypeError as exc: + raise error( + f"{method.__name__}: {exc}\nusage: {call_usage(method)}" + ) from None try: return method(*args, **kwargs) except error: @@ -248,11 +279,21 @@ def push_to_dds( `{dds_path}/{folder}/{target_name}` in DDS — `{uuid}.xml` unless `target_name` is given — after checking `dds_path` exists in the Dremio catalog (skip with `check_catalog=False`).""" + # Arguments first, strictly typed in this method's own parameter order: + # a bad argument is reported as such, before any DDS or Dremio settings + # are looked up. + check_arg_types( + "push_to_dds", + ("dds_path", dds_path, str, False), + ("target_name", target_name, str, True), + ("metadata", metadata, SdiMetadata, True), + ("folder", folder, str, False), + ("force", force, bool, False), + ("check_catalog", check_catalog, bool, False), + ) metadata = metadata or self._last() if metadata is None: raise MetadataSessionError("nothing to push yet — run %sdi get_xml(uuid) first") - # Arguments first: a bad target_name / dds_path / folder is reported as - # such, before any DDS or Dremio settings are looked up. metadata_path(dds_path, metadata.uuid, folder=folder, target_name=target_name) return self._push_controller(with_catalog=check_catalog).push_to_dds( metadata, diff --git a/tests/notebook/test_magics.py b/tests/notebook/test_magics.py index 8605239..5a861f5 100644 --- a/tests/notebook/test_magics.py +++ b/tests/notebook/test_magics.py @@ -515,3 +515,71 @@ def test_metadata_help_lists_methods_without_building_a_session( assert "separated with either '.' or '/'" in out assert "dds_path, target_name=None, metadata=None, folder='metadata'" in table_html assert _magics_instance(ip)._metadata_session is None + + +class _FakePushSession: + """Just the signature `%metadata push_to_dds` exposes — never pushes.""" + + def push_to_dds( + self, + dds_path: str, + target_name: str | None = None, + metadata: object | None = None, + folder: str = "metadata", + force: bool = False, + check_catalog: bool = True, + ) -> str: + return "pushed" + + +USAGE = ( + "usage: push_to_dds(dds_path, target_name=None, metadata=None, folder='metadata', " + "force=False, check_catalog=True)" +) + + +@pytest.mark.parametrize( + ("line", "expected"), + [ + ( + "push_to_dds(P, metadata=m, True)", + "argument 3 (True) has no name, but comes after metadata=m — once an argument is " + "given by name, every argument after it needs a name too. Did you mean force=True " + "or check_catalog=True?", + ), + ( + 'push_to_dds(P, metadata=m, "x.xml")', + 'Did you mean target_name="x.xml" or folder="x.xml"?', + ), + ("push_to_dds(P, metadata=m, flag)", "Did you mean target_name=flag or folder=flag"), + ('push_to_dds("a, b", force=True, "c(,)")', 'argument 3 ("c(,)") has no name'), + ("push_to_dds(P,", "invalid syntax:"), + ], +) +def test_syntax_error_names_the_argument_and_shows_the_signature( + ip: Any, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + line: str, + expected: str, +) -> None: + monkeypatch.setattr(magics_module, "_build_metadata_session", lambda last: _FakePushSession()) + + assert ip.run_line_magic("metadata", line) is None + + out = capsys.readouterr().out + assert out.startswith("metadata error: ") + assert expected in out + assert USAGE in out + + +def test_syntax_error_on_an_unknown_method_has_no_usage( + ip: Any, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.setattr(magics_module, "_build_metadata_session", lambda last: _FakePushSession()) + + ip.run_line_magic("metadata", "nope(a=1, 2)") + + out = capsys.readouterr().out + assert out.startswith("metadata error: invalid syntax:") + assert "usage:" not in out diff --git a/tests/sdi/test_controller.py b/tests/sdi/test_controller.py index 9ea7777..e144dcd 100644 --- a/tests/sdi/test_controller.py +++ b/tests/sdi/test_controller.py @@ -148,6 +148,16 @@ def test_metadata_path_rejects_the_record_as_target_name(sdi: SdiController) -> metadata_path("water", UUID, target_name=record) # type: ignore[arg-type] +def test_controller_push_checks_argument_types(sdi: SdiController) -> None: + record = extracted(sdi) + with pytest.raises(TypeError, match=r"argument 3 \(target_name\) must be str or None"): + sdi.push_to_dds(record, "water", record) # type: ignore[arg-type] + with pytest.raises(TypeError, match=r"argument 1 \(metadata\) must be SdiMetadata, not str"): + sdi.push_to_dds("water", "water") # type: ignore[arg-type] + with pytest.raises(TypeError, match=r"argument 5 \(force\) must be bool, not int"): + sdi.push_to_dds(record, "water", force=1) # type: ignore[arg-type] + + def test_metadata_path_rejects_non_str_uuid() -> None: with pytest.raises(TypeError, match="uuid must be a str"): metadata_path("water", 42) # type: ignore[arg-type] diff --git a/tests/sdi/test_session.py b/tests/sdi/test_session.py index c88f0c2..c17a528 100644 --- a/tests/sdi/test_session.py +++ b/tests/sdi/test_session.py @@ -306,7 +306,9 @@ def test_push_wrong_target_name_type_is_a_session_error() -> None: sdi = SdiSession(env=ENV) sdi.get_xml(UUID) - with pytest.raises(MetadataSessionError, match="target_name must be a file name"): + with pytest.raises( + MetadataSessionError, match=r"argument 2 \(target_name\) must be str or None, not int" + ): _metadata_session(sdi, env={**ENV, **IDENTITY}).push_to_dds( "water", 42, check_catalog=False # type: ignore[arg-type] ) @@ -322,3 +324,41 @@ def test_bad_target_name_is_reported_before_any_settings(bad: str) -> None: with pytest.raises(MetadataSessionError, match="target_name"): no_settings.push_to_dds("water", bad) + + +@pytest.mark.parametrize( + ("args", "kwargs", "message"), + [ + ((42,), {}, r"argument 1 \(dds_path\) must be str, not int"), + (("water", None, "not a record"), {}, r"argument 3 \(metadata\) must be SdiMetadata"), + (("water", None, None, 7), {}, r"argument 4 \(folder\) must be str, not int"), + (("water", None, None, "metadata", 1), {}, r"argument 5 \(force\) must be bool, not int"), + (("water",), {"check_catalog": "no"}, r"argument 6 \(check_catalog\) must be bool"), + ], +) +def test_push_checks_every_argument_type_strictly( + args: tuple[object, ...], kwargs: dict[str, object], message: str +) -> None: + # Reported before anything else, so no record or settings are needed. + with pytest.raises(MetadataSessionError, match=message): + _metadata_session(env={}).push_to_dds(*args, **kwargs) # type: ignore[arg-type] + + +@pytest.mark.parametrize( + ("kwargs", "message"), + [ + ({"colour": "red"}, "got an unexpected keyword argument 'colour'"), + ({"dds_path": "again"}, "multiple values for argument 'dds_path'"), + ], +) +def test_call_that_does_not_fit_the_signature_shows_the_usage( + kwargs: dict[str, object], message: str +) -> None: + with pytest.raises(MetadataSessionError) as info: + _metadata_session(env={}).push_to_dds("water", **kwargs) # type: ignore[arg-type] + + assert message in str(info.value) + assert str(info.value).endswith( + "usage: push_to_dds(dds_path, target_name=None, metadata=None, folder='metadata', " + "force=False, check_catalog=True)" + ) From 1b85d630835804dedf1f6cd16ffd97a73c294c8b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 2 Oct 2026 10:42:48 +0000 Subject: [PATCH 3/4] Bump version to 0.1.23 [skip ci] --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 2633e79..3981ba4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "EEADataLakehouse" -version = "0.1.22" +version = "0.1.23" description = "Data preparation and validation utilities for the EEA data lakehouse pipeline." readme = "README.md" requires-python = ">=3.11" From cb773aea24a8065280078dd440131085b1b05b40 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 2 Oct 2026 10:42:49 +0000 Subject: [PATCH 4/4] Update README install examples to v0.1.23-staging [skip ci] --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f90c812..b9307b0 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@staging" ``` # staging's latest release (early access) — pin to the tag the "staging" badge above shows -pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.22-staging" +pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.23-staging" ``` ## Usage @@ -428,7 +428,7 @@ To pin to one specific release instead, use the exact tag the live badges under %pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.22" # staging's latest release (early access) -%pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.22-staging" +%pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.23-staging" ``` Use the `%pip` magic rather than `!pip` — it installs into the kernel the