[MAINTENANCE] Generalize the data source declaration record beyond SQL backends - #12110
Draft
joshua-stauffer wants to merge 22 commits into
Draft
[MAINTENANCE] Generalize the data source declaration record beyond SQL backends#12110joshua-stauffer wants to merge 22 commits into
joshua-stauffer wants to merge 22 commits into
Conversation
…vocabulary into it Support-tier declarations exist only for SQL backends today, so nothing in the tree can state a tier for a data source that is not SQL. The record that carries those declarations is therefore split: a universal core holding what every data source has, and the SQL-only facts left where they are. The core module imports nothing from the harness, which keeps it leftmost in the package's dependency direction and lets a test construct a throwaway record without importing a data source. A subprocess check enforces that by loading the module with the harness, SQLAlchemy, Spark and every dialect driver blocked. The record is frozen and keyword-only. Keyword-only is what lets a required dialect field on the SQL sub-record follow the core's defaulted fields; every construction site in the tree already passes keywords, so nothing is broken by it. A declared marker resolves to a mark decorator, and a record with no marker raises rather than returning a placeholder. A placeholder would let a data source be parameterized into a suite where it selects nothing and reports as passing. The three moved types keep their current names so this diff reads as a move rather than a move hiding a rename, and the SQL record stays standalone - it reads the moved types from their new home but subclasses nothing yet. No declaration changes.
✅ Deploy Preview for niobium-lead-7998 canceled.
|
… SQL data source SqlBackendSpec now subclasses the universal record and holds only the five dialect facts. Keyword-only construction is what makes the split possible at all: the sub-record's required schema-usage field has to be able to follow the core's defaulted fields. The schema-usage field stays required rather than gaining a default. A backend that forgot to declare it would silently get schema-less table creation, which is a wrong-data failure rather than an error. All thirteen SQL declarations gain a public name, an execution engine and their fluent type names. Public names come from the shipped supported-data-source vocabulary wherever a member exists, so the two never drift apart; the four backends with no member yet supply their own. No other field changes on any of the thirteen. Inheriting an optional marker and CI lane means two consumers must narrow. The identity validator now returns the marker it validated rather than having the registry re-read it, and the lane's emptiness checks apply to a stated lane only - they reject a lane that names nothing, not the absence of one. Scaling obligations to what a record claims is separate work.
A data source can now be registered without a config class, so a record may exist for a data source the harness has no test config for. The class decorator stays for config-bound data sources; a plain function covers the rest. The accessors read live registry state on every call rather than building a list at import, and order by label. A list built at import is only correct while no module registers after it, which is a property of import order rather than of the registry. A record with no config class appears in the record accessors and is absent from the config, tier and engine accessors, because those answer which test configs to parameterize over and a record with no config supplies none. The isolation seam clears storage before yielding rather than only restoring afterwards. Restoring afterwards leaves a test inside the seam observing however many records happen to be registered, so a whole-registry equality assertion would be a statement about the rest of the suite. Registration accepts and rejects exactly what it did before: every validator is unchanged and every message identical. Scaling obligations to what a record claims is separate work.
A record is now validated on its own terms and then against what it claims. Well-formedness covers one record in isolation: a non-empty label and public name, a non-empty marker when one is declared, non-empty lane fields when a lane is declared, and a marker scope with no marker to describe. The obligations that scale are the ones a tier claim creates. A tier claim asserts that a suite runs somewhere, so a record claiming any tier must declare a marker and a lane, and must name a container service if the harness starts it locally. A claim no lane attests to is how a support table starts advertising coverage that never runs. That is also why the container rule relaxes in one direction only. Naming a service without local-container provisioning stays an error, because the service would never be started. Declaring local-container provisioning with no service becomes legal for a record claiming no tier - the honest declaration for a data source distributed as a container image this repository has no compose file for. Marker uniqueness now applies only between records that both declare their marker dedicated. A record declaring its marker shared asserts that the marker names a dependency class, which can legitimately contain another data source. An undeclared scope reads as dedicated: reading it as shared would silently drop collision detection for every record in the tree, since none declares a scope. Label uniqueness applies on both registration paths, so a record with no config class can no longer overwrite a config-bound entry and orphan its marker index.
The declaration lived on the SQL config base, so only a SQL data source could state one. It moves to the shared base, where every config already lives, and the test label and pytest mark become concrete properties derived from it rather than abstract members each config restates by hand. Two declaration slots must never coexist, so the move and the removal of the SQL base's own annotation land together. The type checker does not object to a subclass re-annotating an inherited class variable with a narrower type - that was assumed and is not so - which is why a test walks every class in the package and asserts the slot is annotated in exactly one place. The slot is a ClassVar, so it takes no part in construction, equality or hashing: no config is re-declared and no config's constructor, equality or hash changes. The thirteen modules that assign it are untouched. The SQL base keeps its accessor and its per-instance override, and that accessor is now the single point where the declaration is narrowed to the SQL sub-record. The narrowing is sound because registration rejects a SQL config whose record is not a SQL sub-record, and the accessor's docstring says so, so the cast is not read as unchecked.
Declare the pandas DataFrame config's identity as a core DataSourceSpec and register it, deriving its label and pytest mark from that record instead of from hand-written properties. Registering a non-SQL config class needs an entry point that accepts a core-width record: register_data_source_config. register_sql_backend becomes its SQL-specific wrapper, keeping its dialect-record rejection ahead of the shared rules. The registration protocol's declaration slot is widened by making the SQL-width protocol nominally extend the core-width one - a Protocol variable member is invariant, so a config assigning a sub-record could not otherwise satisfy a protocol demanding the wider type.
Declare this config's identity as a core DataSourceSpec and register it, deriving its label and pytest mark from that record instead of from hand-written properties. It shares its public name with the DataFrame config: two records describing variants of one data source name the same thing to a user, even though the harness exercises them separately. The dataclass declaration is deliberately untouched, and in particular gains no equality opt-out. This config's generated equality compares the read and write option mappings it exists to carry, while the inherited implementation compares only label and mark; since the session batch-setup cache keys on config equality, inheriting it would let a config constructed with non-default read options share a cached setup with a default one and read its CSVs with the wrong options. A suite here constructs exactly that pair.
Declare this config's identity as a core DataSourceSpec and register it, deriving its label and pytest mark from that record instead of from hand-written properties. Provisioning is declared as a local file, not a local container, and the declaration records why. A compose directory exists for this marker and the task runner's entry names it, so a container declaration would satisfy the wiring drift check and still be false: that compose file starts a Spark Connect server so a host without a modern JDK can run Spark tests, while this config starts no server at all - it builds an in-process session and reads CSVs off the local filesystem. A check that passes while asserting something untrue is worse than one that fails, and a later reader who finds the compose file needs the distinction written down. The dataclass declaration is untouched and gains no equality opt-out, for the same reason as the pandas filesystem-CSV config: inheriting the base's implementations would widen equality across the read and write option mappings this config exists to carry.
Add records for the three object stores and the five postgres-compatible and Microsoft Fabric flavors, registered through the config-less entry point. A declaration-only record states what a data source is and how its tests would be selected; it asserts nothing about coverage. The module docstring draws the distinction a reader will otherwise conflate: a declared CI lane means a job installs this data source's dependencies and runs something, while tier membership means a tier's suite passes here. Only the second is a support claim, and none of the eight makes it. Each absence is a statement too. Amazon S3 declares no requirements file because its task-runner entry names the shared lite file rather than an S3-specific one; Azure Blob Storage declares no marker because none exists, and naming one that does not would fail the drift check; Citus declares local-container provisioning with no container service, which is the honest statement that this is how it would run locally and this repository does not.
Both engine-keyed lists were hand-written literals while the SQL lists were already derived. They are now registry queries keyed on the declared execution engine, which leaves every exported name, both tier-derived lists, the combined list and the per-tier-case accessor untouched. The module docstring explained that asymmetry as an unresolved condition, which is no longer true. It now explains why the two derivation keys differ: a tier is a claim about coverage, something a maintainer decides and a backend can join or leave without anything about the backend itself changing, while an execution engine is a fact about the data source. Deriving the pandas list label-orders it, which swaps its two entries. No test id, membership or result changes - position reaches neither a test id nor a marker, both of which come off the config. Two pins in the registry guard module encode the old order and are deliberately left failing here: that module's pins are updated together in a later change, and editing them from here would be reaching into work this one does not own.
…ations Split the registered-set pin into two ordered whole-set equalities, one over every registered record's label and one over the registered config classes, so the existing assertion keeps its shape while records with no config are covered too. Update the derived-list pins to the new pandas label order and say in the docstring what the order now is and why it changed. Pin what six earlier changes left unpinned: the three retrofitted configs' declared fields, against hand-written control records; the marker relaxations and the claim-scaled registration obligations, proven in both directions with throwaway records inside the isolation seam rather than against the real registry, which shares no dedicated marker and so would pass either way; the guarantee that every registering module is imported before any derived list is built, read out of the package's own import order rather than a hand list; and that the real registry survives this module unaltered. Every registered config's equality and hash are pinned to resolve exactly as they did before this work, against a literal captured from that baseline. The stronger form its neighbour uses - that a config's implementations are the shared base's - is deliberately not used here: applied to every config it would force the two filesystem configs off their own generated equality, which compares the read and write option mappings they exist to carry, and two differently-configured instances would then compare equal and share one cached batch setup.
The check parametrized over registered config classes, so a record with no config was never enumerated - which is why declaring eight data sources with no harness config added no cases here at all. It now parametrizes over every registered record. Each assertion fires only on a coordinate the record actually declares. The unconditional lane assertion could not survive that widening as written: a record is allowed to declare no CI lane, and several correctly do. The obligation it encoded moves to where it belongs - a record claiming tier membership must declare both a marker and a lane, because a tier claim asserts that a suite runs somewhere. The task-runner assertion splits in two: that the entry exists when a task-runner marker is declared, and that it lists the requirements file only when both are declared. Without the split, a record declaring a task-runner marker and no requirements file asserts that nothing is among that entry's files, and fails - a shape that exists today. Two ghost records join the failure-path tests, since a conditional never observed failing is not evidence that it fires.
Assert that every member of the shipped supported-data-source vocabulary resolves to at least one registered record carrying its exact value as a public name, and pin the registered names that have no member as an explicit literal. The check is deliberately one-directional. Requiring every record to have a member would fail immediately for eight data sources, and closing that gap is not this work's to close: that vocabulary is a public metadata surface in the shipped package, so adding a member to it is a product decision about what the package advertises, not something a test harness gets to force. The pinned literal is what stops the one-directional check from becoming a silent ratchet - if a member is added upstream for one of the eight, the literal drifts and this check says so. 'Resolves to at least one record' cannot stand alone: two records deliberately share the public name Pandas, so dropping either one leaves the forward check green while a variant has silently stopped being exercised. A second assertion pins every variant a member names.
…cords Each of the three already runs every module of the shared canonical expectation parameterization - in the unit, filesystem and Spark lanes - and each already declares the marker and CI lane a tier claim obliges. Declaring the tier therefore states a result the tree already produces, and edits no configuration file. The combined list stops being three derivations added together and becomes one read of that declared claim, with the SQL list its intersection with the SQL execution engine. Both halves land together: between them the SQL list would hold the pandas and Spark configs. Membership does not move on any derived list, checked against the memberships recorded before this work rather than against the new code, and the selected test ids are unchanged in every consuming lane. The combined list is now label ordered rather than grouped by engine; nothing reads it positionally. Three wiring cases that skipped with 'claims no tier' now run and pass.
Registration now rejects a record that carries a config class and a declared execution engine but does not declare the criterion, unless its label appears in an explicit literal of deliberate non-participants that states a reason per entry. That literal holds the four curated-tier backends and nothing else. The rejection says why: a config the harness drives against a named engine runs that suite unless someone decided otherwise, and that decision has to be written down with its reason. Silent omission is how three SQL backends came to be missing from the suite. The literal carries two liveness checks - it fails if it names a label that resolves to no registered record, and if it names a record that does declare the criterion - because without them it is a place exemptions accumulate after their reason expires. All three rules are proven with throwaway records inside the isolation seam. Every registered config already satisfies the rule, so an assertion over the real registry would pass whether or not the rejection fires. The decisive case is a near-miss: a record identical to the rejected one but for a label the literal names registers cleanly, so the exemption is the only thing admitting it. The generic-SQL escape hatch never reaches this rule because it is never registered - and it must not be exempted, since an exemption naming it would fail the liveness check that requires an exempted label to resolve to a registered record.
Two hand-written lists carried the same names as the derived pair while holding different membership, which is how three SQL backends came to run the full expectation suite while sitting outside every list that gates CI. Now that the two definitions agree, the duplicates are deleted and their consumers repointed at the derived names rather than renamed into a second home - a rename would have removed the collision while preserving the difference the collision made visible. Three lists that are not registry facts move to a non-test module in the same package. The non-SQL one is derived from the execution engine, which reproduces its membership exactly; the other two stay declared, each recording why it is not derived and what would make derivation possible. The derived pair loses exactly one entry, the generic-SQL escape hatch, which carries no registered record and so cannot be derived. That reduction is asserted as a set difference in both directions rather than as a new count, so a second entry cannot disappear unnoticed. It changes what executes in no lane: every marker a workflow selects collects exactly what it collected before, and the marker the hatch declares is selected by no lane at all - which is the basis of the removal. The equality check between the derived and hand-written lists is retired here rather than earlier, because it loses its second term with the list it compared against. Its invariant is carried by the mandatory shared parameterization criterion, which makes the same divergence a registration error, and by pins fixed against the membership captured before anything moved.
…ibes The record, the registry and the tier enumeration stopped being SQL-only several changes ago; their names had not caught up. Nothing that describes a data source in general says 'backend' any more, and 'SQL' survives only where what is named exists to prove dialect behavior. The shared parameterization tier member is the important one. It named an execution engine while describing a suite that pandas, Spark and SQL data sources all run, and that mismatch is why the derived-list module had to concede an asymmetry the record had already resolved. Its value moves with its name; the same records declare it before and after. The curated member keeps saying SQL, because the suite it gates exists to prove dialect behavior and means nothing for a non-SQL data source - which is the test for whether a name should keep saying SQL. So do the SQL sub-record, the SQL config base, the transaction mode enumeration and the schema-item factory. No compatibility alias is left behind; an alias would create the second name this rename exists to remove. The SQL config base also loses its label and mark overrides, which duplicated the concrete base properties and resolved identically through the same override seam. No behavior changes. The expectation suite's collected identifiers are byte-identical, and the two verification modules' identifiers differ only by the renamed test classes, one for one.
Both modules verify every registered data source, not only SQL backends, and their filenames were the last place still saying otherwise. Moved with git mv so history follows them; their contents are unchanged. Every reference moves with them, including the walkthrough's instructions for running each check. Each module keeps exactly one project marker - marker coverage is a blocking gate, and a module that loses its marker fails collection rather than a test.
The walkthrough described onboarding a SQL backend, which is no longer what the harness does. It now covers declaring a data source of any kind, keeping its step order and demoting the dialect extension points to a subsection that applies when the record is a SQL sub-record. New material: what a declaration-only record asserts and what it does not, and why declaring an untested data source beats omitting it; dedicated versus shared markers, and what to do when a data source has no marker; that a tier claim obliges a marker and a CI lane, with the reason - a tier is a claim that a suite runs somewhere, and a claim no lane attests to is how a support table starts advertising coverage that never runs; the record schema as a published contract, field by field; and the shared parameterization criterion. The failure tables now carry a row and a remedy for all thirty rejections, enumerated from the registry rather than from memory. The previous text also could not be followed: it never mentioned five of the record's fields, so its own example declaration would not construct today, and its dialect enumerations had drifted from the tree - naming BigQuery as schema-scoped when it declares otherwise, omitting Trino and Oracle from override lists, and calling the guarded-import shape hypothetical when ClickHouse ships it.
Pure move so history follows it; the content changes land separately.
The six backends this document tracked are one shape of gap: the shipped package has a dialect entry and the harness does not test them. The data sources declared here are a different shape - shipped, publicly documented fluent classes with no test surface at all - so 'how to read an entry' now covers both, and each declared-but-untested data source has an entry saying what onboarding would concretely require. Also recorded: the public names the compatibility reference omits, so the omission is visible to whoever regenerates that page; two gaps in the shipped package this work does not close; and one newly observed gap where a required marker is absent from the declared marker list, so marker coverage accepts a test that pytest refuses to collect. The config equality and hash defect goes in as one entry carrying both halves. Two configs are unhashable because they re-declare themselves as frozen dataclasses, and the remedy the harness prescribes for SQL configs would inherit an equality ignoring their read and write option mappings - making two differently-configured instances compare equal and share one cached batch setup, which a suite in this tree would hit today. Split across two entries, a maintainer who found the first would apply the fix that causes the second. Oracle's entry is retired rather than carried forward: it claimed no onboarding surface at all, and Oracle now has every one.
One pointed at the wrong module: tier_case_exclusions is declared on the core record, not on the SQL sub-record, and backend_spec.py does not mention it - so a reader following the reference found nothing and had no way to tell whether the field or the pointer was wrong. The other described the change that removed the duplicated lists in terms of a choice rather than its effect.
joshua-stauffer
force-pushed
the
f/datasource-spec/generalize-record
branch
from
August 28, 2026 02:58
3487415 to
8c801ed
Compare
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
1 participant
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.
Summary
Support-tier declarations existed only for SQL backends. This splits the declarative data source record into a universal core plus the SQL-only dialect facts, so a data source that is not SQL can carry a declaration at all — then declares every data source the harness knows about, derives the lists that gate CI from those declarations, and renames the vocabulary to match what it now describes.
data_source_spec.pyholdsDataSourceSpec: 14 fields, of which onlylabel,public_nameandprovisioningare required. Frozen and keyword-only, importing nothing from the harness, so a test can construct a throwaway record with no dialect driver or Spark distribution installed.SqlBackendSpecis a sub-record holding only the five dialect facts. All thirteen SQL declarations gained a public name, an execution engine and their fluent type names, with no other field changed.Why
Ten of the eighteen data sources on the public compatibility reference had no tier-declaring record anywhere in the tree, so nothing could state what tier they were in or why. Half the advertised surface could not be answered from code.
The concrete failure this prevents already happened: two lists named
ALL_DATA_SOURCESandSQL_DATA_SOURCESexisted in different modules with different membership, and three SQL backends ran the full expectation suite while sitting outside the lists that gate coverage. Silent omission is how that happens, so declaring the criterion is now mandatory and opting out requires writing down a reason.Three design points carry reasons worth stating:
A record with no marker raises rather than returning a placeholder mark. A placeholder would let a data source be parameterized into a suite where it selects nothing — and report as passing. Silence that looks like success is worth spending an exception on.
An undeclared marker scope reads as dedicated. Reading it as shared would make the duplicate-marker rule vacuous for every record in the tree, since none declares a scope — the check would be silently dead with every test still green.
A declared CI lane is not a support claim. A lane means a job installs this data source's dependencies and runs something; tier membership means a tier's suite passes here. Only the second says anything about coverage, and the walkthrough now draws that line explicitly.
User impact
None. This is test-harness infrastructure. No file under
great_expectations/is modified — the one module read there, the shipped supported-data-source vocabulary, is read and never written. No pytest marker, required-marker entry, requirements file, task-runner entry, CI workflow job or matrix entry, or compose service definition was added, removed or changed.How to review
The load-bearing claim is that existing behavior is unchanged, and it is checkable rather than asserted:
developtoday.Three things worth a second opinion:
ClassVarwith a narrower type is a second declaration of one fact, and mypy accepts it with no diagnostic.TestTheDeclarationSlotIsDeclaredExactlyOncewalks every class in the package instead. If you move that slot, keep that test.backend_specnarrows with a cast, sound only because registration rejects a SQL config whose record is not a SQL sub-record. Check that it reads as guarded rather than unchecked.Two known defects are recorded and deliberately not fixed, both in the backlog document: two configs are unhashable because they re-declare themselves as frozen dataclasses, and the remedy the harness prescribes for SQL configs would inherit an equality ignoring their read and write option mappings — making two differently-configured instances compare equal and share one cached batch setup. Both halves are recorded together so that whoever rediscovers the first does not apply the fix that causes the second.
Every new assertion was mutation-tested. Verified locally: pandas, both filesystem sources, Spark (classic and Connect), SQLite, PostgreSQL, MySQL, SQL Server, Trino, ClickHouse, Oracle. SingleStore relies on CI here (a local port conflict), as do the four credential-provisioned backends — BigQuery, Databricks, Snowflake and Redshift.
Known flake:
tests/datasource/fluent/test_batch_parameter_normalization.pyfails intermittently on Python 3.13 from warning-state contamination between co-resident tests. It reproduces ondevelopand is unrelated to this branch — a re-run is the right response.