Repository navigation
feat!: declare models by overriding accessors, not by dunder - #19
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. Includes the two forms the class-body sweep could not see: consumers that read the removed dunders now call the accessor, and runtime assignments that used to declare a table name or schema dynamically now install the classmethod an override would declare.
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.dev3 #19 +/- ##
====================================================
Coverage 77.47% 77.47%
====================================================
Files 140 140
Lines 9826 9826
Branches 1401 1401
====================================================
Hits 7613 7613
Misses 1786 1786
Partials 427 427
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: 19 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.