Repository navigation
feat!: declare models by overriding accessors, not by dunder - #71
Merged
Merged
Conversation
Breaking, following rhosocial-activerecord: model fixtures and examples now override table_name(), primary_key(), pk_auto_generated(), read_only() and the locator methods instead of assigning __table_name__ and friends. The framework stopped reading those dunders, so a model still declaring them would fail with NotImplementedError at the first read. Produced by the core repository's declaration sweep (scripts/ d5_migrate_declarations.py), so every repository in the family carries the same shape. No new lint findings: the totals drop where the removed assignments took their imports with them. CI for this branch installs the family from the matching feature/declaration-surface ref, so a run exercises one revision across core, testsuite and the backend under test rather than a mixture of two.
…rn None The helper that supplies the census' arguments was called _synthesize_arguments: "synthesize" names the act of producing something, not what is produced, for whom, or by what rule -- a reader at the call site cannot say what will be passed. Its rule is concrete and the module already says "probe" everywhere else (PROBE_TABLE, PROBE_COLUMN, PROBE_OBJECT), so the name now says it: _probe_arguments_for. Its return annotation was dict | None, which was wrong twice. Nothing in the function returns None -- every branch fills kwargs and the only return is that dict -- so the annotation misdescribed the contract, and evaluated at definition time it also made the module unimportable on Python 3.8 and 3.9 (PEP 604 arrived in 3.10). It is Dict[str, Any] now, which states what the function does and imports on every version this package supports. The assertion message and the docstring follow the new name, so a reader who trips the "un-askable family" guard is pointed at a helper they can recognise.
…erver error The census accepts two answers: a family that answers, and a family that refuses by a typed exception. It only recognised IntrospectionNotSupportedError, so on PostgreSQL 9 the partition family -- which is gated by the dialect with UnsupportedFeatureError before any statement is sent -- was recorded as "a catalog statement this release does not serve". That reading is wrong twice: the statement was never sent, and the refusal is the contract working as designed. Both classes are now accepted and the group key still separates them, because each names which layer refused: the dialect for a capability the server does not have, the introspector for a family it does not serve. The walk's "un-askable family" guard is gone with it. _probe_arguments_ for always returns a mapping -- an uncovered parameter keeps its own default -- so the guard could never fire, and a check that cannot fail claims protection it does not give. The docstring now says what the code does: every family is asked, and a family that answers nothing is a fact about the backend rather than about the walk.
Workflow 'test.yml' installs this backend's dependencies from the other repositories in the family. Core's PR #140 is merged into release/v1.0.0.dev30, so the core ref now names that branch: a run of this repository tests the released core rather than a feature branch that no longer exists. The testsuite ref is unchanged and still names feature/declaration-surface: its migration has no merge yet, and pointing it at release/v1.0.0.dev17 would install fixtures that still declare the removed dunders.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v1.0.0.dev17 #71 +/- ##
========================================================
+ Coverage 78.07% 78.58% +0.51%
========================================================
Files 360 360
Lines 19340 19399 +59
Branches 3172 3183 +11
========================================================
+ Hits 15099 15244 +145
+ Misses 3109 3017 -92
- Partials 1132 1138 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The testsuite's PR #32 is merged into release/v1.0.0.dev17, so the testsuite ref now names that branch. Every cross-repository install in a run therefore resolves to a released artifact: core from release/v1.0.0.dev30, the testsuite from release/v1.0.0.dev17, and this repository from its own feature branch.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follows rhosocial/python-activerecord#140, which makes a model declare its
table name, key, locators and read-only-ness by overriding an accessor
classmethod instead of assigning a class-body dunder. This repository's own
models -- fixtures, providers, examples and embedded model source -- still
declared
__table_name__and friends, which the released core no longerreads, so every model here was rewritten by the same migration sweep.
What changed in this repository: 200 declaration(s) rewritten to the
accessor form, plus the two forms the class-body sweep cannot see:
Indexed.__table_name__) now callthe accessor (
Indexed.table_name());(
Author.__table_name__ = "archive") now install the classmethod an overridewould declare (
Author.table_name = classmethod(lambda cls: "archive")).No new lint findings: the totals drop where the removed assignments took their
imports with them.
Type of change
feat: A new featuretest: Adding missing tests or correcting existing tests (for the new feature)ci: Changes to our CI configuration files and scripts (if related to the new feature)docs: Documentation only changes (if related to the new feature)perf: A code change that improves performance (if part of the feature)chore: Changes to the build process or auxiliary tools and libraries suchs as documentation generation (if related to the new feature)build: Changes that affect the build system or external dependencies (if related to the new feature)revert: Reverts a previous commitBreaking Change
Impact on End-Users:
N/A for this repository's published API: the change is in its fixtures,
providers and examples, which follow core's declaration surface.
Impact on Backend Developers (Custom Implementations):
A backend that ships models declaring the removed dunders must switch to the
accessors, and a backend handler must extend
DDLAnnotationHandler.annotation_typesrather than replace it --UsePartitionKeyis part of the core tuple now.Related Repositories/Packages
rhosocial/python-activerecord-- PR #140, merged intorelease/v1.0.0.dev30; this repository'stest.ymlinstalls core from thatbranch.
rhosocial/python-activerecord-testsuite-- the fixture migration, onfeature/declaration-surface. Its PR must merge before this one: thetestsuite ref in
test.ymlstill names that branch, becauserelease/v1.0.0.dev17does not yet contain the migration.feature/declaration-surface.Testing
Test Plan:
The full workflow was run on this branch before the ref update: every
Python version × database version job passed. After the ref update the same
matrix runs again, installing the released core.
Checklist
README.md, etc.).changelog.d/<issue_number>.<type>.md).after the PR is opened.
Additional Notes
Migration tooling lives in core's
scripts/(d5_migrate_declarations.py,p3_fix_reads_and_writes.py): the sweep is AST-located rather thanregex-matched, preserves line endings and spacing, and handles the
type(...)namespace-dict form separately from class bodies.Extra commits on this branch
Two commits fix the introspection census this migration surfaced (both in
tests/.../introspection/test_introspection_family_census.py):_synthesize_arguments-- an act, not a rule -- and annotateddict | None, which no branch returns and which made the module unimportable on Python 3.8/3.9. It is_probe_arguments_for(...) -> Dict[str, Any]now.IntrospectionNotSupportedErroras a typed refusal, so a dialect capability gate (UnsupportedFeatureError, raised before any statement is sent) was reported as a broken catalog statement. Both are accepted now, and the guard that could never fire is gone.Both were found by this branch's CI and are unrelated to the declaration migration except that the census is what exercises the introspection surface.