diff --git a/docs/pages/api/exceptions.md b/docs/pages/api/exceptions.md index 35672a9c..d02d10b4 100644 --- a/docs/pages/api/exceptions.md +++ b/docs/pages/api/exceptions.md @@ -22,8 +22,9 @@ FerroError └── MigrationRefused a migration call refused; .report says how far it got ├── PendingMigrationsError ├── DatabaseAheadError - ├── AlreadyTrackedError - └── AppliedAboveBaselineError + └── RunRefused a migration run, or connect(auto_migrate=True) on a tracked database, refused + ├── AlreadyTrackedError + └── AppliedAboveBaselineError ``` `ForeignKeyViolationError` covers every foreign-key rejection: a dangling diff --git a/docs/pages/api/migrations.md b/docs/pages/api/migrations.md index 2fab02d9..efb95134 100644 --- a/docs/pages/api/migrations.md +++ b/docs/pages/api/migrations.md @@ -77,6 +77,8 @@ except ferro.migrations.DatabaseAheadError as ahead: ::: ferro.migrations.DatabaseAheadError +::: ferro.migrations.RunRefused + ::: ferro.migrations.AlreadyTrackedError ::: ferro.migrations.AppliedAboveBaselineError diff --git a/docs/pages/guide/schema/overview.md b/docs/pages/guide/schema/overview.md index 7bd7183b..7281d0ed 100644 --- a/docs/pages/guide/schema/overview.md +++ b/docs/pages/guide/schema/overview.md @@ -47,7 +47,7 @@ A database is managed by auto-migrate **or** by migrations, never both. Once a d RunRefused: connect(auto_migrate=…) is refused: main is governed by ferro migrations (main._ferro_migrations). Use ferro migrate up, or drop the tracking tables to leave migrations. ``` -Nothing has run, and the message names the two ways out. `main` is SQLite's schema; on Postgres the message names the schema the connection works in, such as `public`. +Nothing has run, and the message names the two ways out. The error is a [`RunRefused`](../../api/migrations.md#ferro.migrations.RunRefused), a subclass of `MigrationRefused`, so `except ferro.migrations.MigrationRefused` catches it too. Only some refusals carry a kind, and this one does not: its `kind` is `None`, so match it on its type or its message. `main` is SQLite's schema; on Postgres the message names the schema the connection works in, such as `public`. The rule is per database, not per project. A throwaway test database built with `connect(url, auto_migrate=True)` still works in a project that has migrations, because that database has never run one. diff --git a/docs/pages/upgrading.md b/docs/pages/upgrading.md index 7cd68b82..a806df8e 100644 --- a/docs/pages/upgrading.md +++ b/docs/pages/upgrading.md @@ -71,12 +71,13 @@ If you are moving from `0.12` or earlier straight past `0.14`, the deprecated AP ## Upgrading to 0.26 -Four changes in `0.26` can reach code that runs today. Two show up in a type checker or an import, two in text you compare: +Five changes in `0.26` can reach code that runs today. Two show up in a type checker or an import, two in text you compare, and one in the values a secret column stores: - `Session.query` is now typed: it takes `type[M]` and returns `Query[M]`, so a type checker may flag a call that passes a class that is not a `Model`. - The undocumented module `ferro.migrations.chunked` is now the private `ferro.migrations._chunk_loop`. `from ferro.migrations import chunked` still imports the [`chunked`](api/migrations.md#ferro.migrations.chunked) decorator, unchanged. - The `RuntimeError` an operation raises with no database route ends differently. It said "Implicit default-connection routing was removed in v0.13 (deprecated since v0.12); see the sessions migration guide." and now says "Implicit default-connection routing was removed in v0.14 (deprecated since v0.12); see the upgrade guide." Code that matches the message on its old ending needs the new one; a match on its start, `No database route`, still holds. -- `save()` of a new or fetched record, `save(only=...)`, `save(exclude=...)` and `bulk_create()` list their columns in the model's field order. Before, a `bytes` or secret column came last, a fetched record's `save()` followed the order its `SELECT` returned, and `save(only=...)` or `save(exclude=...)` could change order from one run to the next. The rows written are the same; the statement text is not, so a test or a log check that compares captured SQL (the [statement log](guide/connections.md#logging) or `.sql()`) needs its expected text updated. `update()` is unchanged: it lists the columns in the order the call passes them, a `bytes` or secret column included. +- `save()` of a new or fetched record, `save(only=...)`, `save(exclude=...)` and `bulk_create()` list their columns in the model's field order. Before, a `bytes` column came last, a fetched record's `save()` followed the order its `SELECT` returned, and `save(only=...)` or `save(exclude=...)` could change order from one run to the next. The order changes no value written; the statement text changes, so a test or a log check that compares captured SQL (the [statement log](guide/connections.md#logging) or `.sql()`) needs its expected text updated. `update()` is unchanged: it lists the columns in the order the call passes them, a `bytes` column included. +- A `SecretStr` or `SecretBytes` field now stores its real value, and a fetched record loads it back as a `SecretStr` or `SecretBytes`. `0.25` wrote the mask `**********` in its place on `save()`, `bulk_create()` and `update()`, and loaded the `str` `**********` back. A row `0.25` saved holds the mask, and the real value cannot be recovered from it: write the value again from its source. A `SecretBytes` column is now binary (`BYTEA` on Postgres, `BLOB` on SQLite); `0.25` created it as `varchar`. Change such a column to the binary type with a migration (`ferro migrate new` writes it) before `0.26` saves to it: on Postgres a `SecretBytes` value saved into the `varchar` column is stored as its hex text (`\xff006b` for `b"\xff\x00k"`) and loads back as those characters. ## Upgrading to 0.25 diff --git a/src/ferro/migrations/__init__.py b/src/ferro/migrations/__init__.py index c5a2995d..105bd7cf 100644 --- a/src/ferro/migrations/__init__.py +++ b/src/ferro/migrations/__init__.py @@ -31,7 +31,7 @@ from ._drift import DriftReport, drift, render_op from .errors import DatabaseAheadError, MigrationRefused, PendingMigrationsError from .generate import check -from .report import AlreadyTrackedError, AppliedAboveBaselineError +from .report import AlreadyTrackedError, AppliedAboveBaselineError, RunRefused from .runner import require_applied, status, up from .steps import atomic, chunked, irreversible, nothing_to_reverse, todo @@ -46,6 +46,7 @@ "DriftReport", "MigrationRefused", "PendingMigrationsError", + "RunRefused", "atomic", "baseline", "check", diff --git a/src/ferro/migrations/report.py b/src/ferro/migrations/report.py index 7b6632f4..6967e518 100644 --- a/src/ferro/migrations/report.py +++ b/src/ferro/migrations/report.py @@ -47,10 +47,23 @@ class RunRefused(MigrationRefused): """A migration run's refusal before it ran anything, naming the fix. Raised for every refusal the run planner, the run lock and the tracking - tables make; its message is the operator's text. A planner refusal also - carries what it is about, so a caller matches on `kind` rather than on - the text. A kind with a class of its own (`AlreadyTrackedError`, - `AppliedAboveBaselineError`) arrives as that class. + tables make, and by `connect(auto_migrate=True)` on a database that + migrations manage; its message is the operator's text. It is a + `MigrationRefused`, so `except MigrationRefused` catches it too. Some + refusals also carry what they are about in `kind`, so a caller matches + on it rather than on the text; on the others `kind` is `None`. A kind with a class of its own + (`AlreadyTrackedError`, `AppliedAboveBaselineError`) arrives as that + class. + + Examples: + ```python {test=compile reason="needs a project with a migrations directory"} + try: + await ferro.migrations.up() + except ferro.migrations.RunRefused as refused: + if refused.kind == "edited_applied": + print(refused) # names the applied migration that was edited + raise + ``` """ def __init__( @@ -68,10 +81,12 @@ def __init__( ) -> None: super().__init__(message) self.kind = kind - """The planner's refusal kind (`"irreversible"`, `"edited_applied"`, ...), or `None`. + """The refusal's kind (`"irreversible"`, `"edited_applied"`, ...), or `None`. - `None` for a refusal that is not the planner's (a lost lock, a - missing schema). + Only some refusals carry a kind. `connect(auto_migrate=True)` on a + database that migrations manage, a lost run lock and a data step + that does not load are among those whose `kind` is `None`; match + those on the message, or catch the `MigrationRefused` base. """ self.migration = migration """The migration number the refusal names, or `None`.""" diff --git a/tests/test_docs_output.py b/tests/test_docs_output.py index d685434f..dbe9c618 100644 --- a/tests/test_docs_output.py +++ b/tests/test_docs_output.py @@ -85,19 +85,65 @@ def test_a_postgres_text_block_is_what_its_example_prints_there( ) -def _ferro_exception_names() -> list[str]: - """The name of every exception class Ferro defines.""" +def _defined_exception_classes() -> set[type]: + """Every exception class Ferro defines, internal ones included.""" import ferro.migrations import ferro.migrations._chunk_loop # noqa: F401 - defines BatchFailed - found: set[str] = set() + found: set[type] = set() pending: list[type] = [BaseException] while pending: cls = pending.pop() pending.extend(cls.__subclasses__()) if cls.__module__.split(".")[0] == "ferro": - found.add(cls.__name__) - return sorted(found, key=lambda name: (-len(name), name)) + found.add(cls) + return found + + +def _public_exception_names() -> set[str]: + """The exception classes a reader can import by the name a page prints: + those in ``ferro.__all__`` or ``ferro.migrations.__all__``, and the public + classes ``ferro.exceptions`` defines.""" + import ferro + import ferro.exceptions + import ferro.migrations + + exported = { + exported + for module in (ferro, ferro.migrations) + for name in module.__all__ + if isinstance(exported := vars(module).get(name), type) + } + return { + cls.__name__ + for cls in _defined_exception_classes() + if cls in exported + or (cls.__module__ == "ferro.exceptions" and not cls.__name__.startswith("_")) + } + + +def _ferro_exception_names() -> list[str]: + """The name of every public exception class Ferro defines, longest first.""" + return sorted(_public_exception_names(), key=lambda name: (-len(name), name)) + + +def _internal_exception_names() -> set[str]: + """The exception classes Ferro defines that no public path exports.""" + return {cls.__name__ for cls in _defined_exception_classes()} - set( + _public_exception_names() + ) + + +_EXCEPTION_LINE = re.compile(r"(?m)^\s*(?:[^\s:]+:\d+: )?(?:[\w.]*\.)?(\w+): ") +"""A line opening with an exception's name, as ``print`` of a raised error or +a traceback's last line shows it.""" + + +def internal_exception_names_in(body: str) -> list[str]: + """The internal Ferro exception classes a block prints by name; a reader + who writes ``except`` with one of them gets a ``NameError``.""" + internal = _internal_exception_names() + return [name for name in _EXCEPTION_LINE.findall(body) if name in internal] FERRO_OUTPUT = re.compile( @@ -195,6 +241,33 @@ def test_the_recogniser_matches_every_block_of_ferro_output(block: Fence) -> Non ) +@pytest.mark.parametrize("block", page_fences("text"), ids=page_name) +def test_a_printed_exception_name_is_one_a_reader_can_import(block: Fence) -> None: + internal = internal_exception_names_in(block.body) + assert not internal, ( + f"{page_name(block)}: the block prints {internal}, which neither " + "ferro nor ferro.migrations exports; export and document the class, " + "or print its nearest public base" + ) + + +def test_a_block_naming_an_internal_exception_class_fails() -> None: + assert "StepRefused" in _internal_exception_names() + assert internal_exception_names_in( + "Step refused:\nStepRefused: the step needs a reason\n" + ) == ["StepRefused"] + assert internal_exception_names_in( + "ferro.migrations.steps.StepRefused: the step needs a reason\n" + ) == ["StepRefused"] + assert internal_exception_names_in("MigrationRefused: refused\n") == [] + + +def test_the_recogniser_names_only_public_exception_classes() -> None: + names = set(_ferro_exception_names()) + assert "MigrationRefused" in names + assert not names & _internal_exception_names() + + def test_each_varies_entry_names_a_shows_block() -> None: shows = { page_name(block) @@ -350,3 +423,28 @@ def test_a_block_under_a_postgres_fence_holds_what_it_prints_there( return with pytest.raises(AssertionError, match="differs from what the fence at line 3"): check_printed(block, dialect="postgres", database_url=db_url) + + +def test_the_pages_say_only_some_refusals_carry_a_kind() -> None: + """``RunRefused.kind`` is ``None`` on many refusals (the tracked-database + refusal of ``connect(auto_migrate=True)`` among them); no page or + docstring may say it always names the refusal.""" + import ferro.migrations + + pages = REPO_ROOT / "docs" / "pages" + overview = (pages / "guide/schema/overview.md").read_text(encoding="utf-8") + tree = (pages / "api/exceptions.md").read_text(encoding="utf-8") + [run_refused] = [line for line in tree.splitlines() if "RunRefused " in line] + import inspect + + source = inspect.getsource(ferro.migrations.RunRefused.__init__) + kind_doc = source.split("self.kind = kind", 1)[1].split('"""', 2)[1] + for text in (overview, run_refused, kind_doc): + assert "names why" not in text + assert "names its refusal in `kind`" not in text + assert "connect(auto_migrate=True)" in run_refused + assert "Only some refusals carry a kind" in overview + assert "its `kind` is `None`" in overview + assert "Only some refusals carry a kind" in kind_doc + assert "connect(auto_migrate=True)" in kind_doc + assert "MigrationRefused" in kind_doc diff --git a/tests/test_save_column_order.py b/tests/test_save_column_order.py index f9d0ba44..0cd41a06 100644 --- a/tests/test_save_column_order.py +++ b/tests/test_save_column_order.py @@ -230,3 +230,30 @@ def test_the_upgrade_guide_states_each_write_door_s_column_order(): "`update()` is unchanged: it lists the columns in the order the call passes them" in rest ) + + +def _upgrade_section() -> str: + guide = ( + Path(__file__).resolve().parents[1] / "docs/pages/upgrading.md" + ).read_text() + return guide.split("## Upgrading to 0.26", 1)[1].split("\n## ", 1)[0] + + +def test_the_upgrade_guide_states_the_secret_column_change(): + """0.25 wrote a secret column as the mask, in field order, and made a + ``SecretBytes`` column ``varchar``; the 0.26 note says so and says what + a 0.25 user must do.""" + section = _upgrade_section() + assert "secret column came last" not in _upgrade_bullet() + [bullet] = [line for line in section.splitlines() if "`SecretStr`" in line] + for fact in ( + "`SecretBytes`", + "`**********`", + "cannot be recovered", + "`varchar`", + "`BYTEA`", + "`BLOB`", + "migration", + ): + assert fact in bullet, fact + assert section.split(":", 1)[0].strip().startswith("Five changes")