[TIDY-FIRST] Type the runners - Part 2#12810
Merged
Conversation
Contributor
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
QMalcolm
changed the title
3 tasks
Contributor
Author
|
This is a stacked PR |
2 tasks
QMalcolm
force-pushed
the
qmalcolm--tidy-first-type-the-runners
branch
from
ce3f829 to
43f38ea
Compare
QMalcolm
force-pushed
the
qmalcolm--tidy-first-type-the-runners-part-2
branch
from
122b135 to
6c831bb
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12810 +/- ##
=======================================
Coverage 91.36% 91.37%
=======================================
Files 203 203
Lines 25884 25881 -3
=======================================
- Hits 23649 23648 -1
+ Misses 2235 2233 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
MichelleArk
previously approved these changes
Apr 10, 2026
…tion BaseRunner is now Generic[NodeT, RunnerResultT] where: - RunnerResultT (existing) = the result type (RunResult, FreshnessNodeResult, etc.) - NodeT (new) = the node type the runner operates on (ModelNode, SourceDefinition, etc.) self.node is now typed as NodeT instead of ResultNode, which will allow subclasses to declare their concrete node type and eliminate all [union-attr] ignores on self.node attribute accesses. The abstract execute() now takes compiled_node: NodeT so subclass overrides with narrower types (ModelNode, FunctionNode, etc.) no longer violate LSP. A single type: ignore[arg-type] in run() bridges the ResultNode from ExecutionContext to the NodeT that execute() expects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erride ignores - CompileRunner(BaseRunner[CompilableNodeT, RunResult]): CompilableNodeT is a new TypeVar bound to ManifestSQLNode, allowing compile_node(self.node, ...) to be called without type: ignore[arg-type] - ModelRunner(CompileRunner[ModelNode]): self.node is now ModelNode, removing all 8 union-attr ignores on ModelNode-specific attributes in run.py (language, config.batch_size, config.concurrent_batches, has_this, batch, previous_batch_results) and the override ignore on MicrobatchModelRunner.execute - FunctionRunner(CompileRunner[FunctionNode]): removes the __init__ workaround (assert isinstance + self.node reassignment) and the override ignore on execute - ShowRunner(CompileRunner[ManifestSQLNode]): ShowRunner processes any compileable node type Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… typing FreshnessRunner: - Now BaseRunner[SourceDefinition, FreshnessNodeResult]: removes union-attr on self.node.source_name (before_execute) - after_execute simplified: use self.node.source_name/name directly instead of hasattr(result, "node") branch (which was always true since both PartialSourceFreshnessResult and SourceFreshnessResult inherit node from NodeResult); removes 3 union-attr ignores TestRunner: - UnitTestDefinition is not in ManifestSQLNode/ResultNode so TestRunner cannot be fully specialized without a node-type hierarchy change; use CompileRunner (type: ignore[type-arg]) to keep existing behavior - describe_node_name: replace resource_type == NodeType.Unit check with proper isinstance(self.node, UnitTestDefinition) — removes attr-defined ignore - execute: corrects type from Union[TestNode, UnitTestNode] to Union[TestNode, UnitTestDefinition] (model/versioned_name are on UnitTestDefinition, not UnitTestNode) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sult] These runners operate on any refable/generic node type, so ResultNode is the correct NodeT — no node-specific attribute accesses to narrow further. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
freshness.py: - _metadata_freshness_cache: retype from Dict[BaseRelation, FreshnessResult] to Dict[BaseRelation, FreshnessResponse] (FreshnessResponse is the TypedDict with age/max_loaded_at/snapshotted_at that the code actually stores); removes assignment and index ignores on freshness cache lookup and freshness["age"] - adapter_response: use separate adapter_response_dict variable instead of reassigning adapter_response from AdapterResponse to Dict; removes assignment and arg-type ignores build.py: - RUNNER_MAP.get() returns ABCMeta | None which doesn't match Optional[Type[BaseRunner[Any, Any]]] after BaseRunner became generic; suppress with type: ignore[return-value] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
QMalcolm
force-pushed
the
qmalcolm--tidy-first-type-the-runners-part-2
branch
from
6c831bb to
96eb75e
Compare
Contributor
Author
|
Rebased off of main after the squash merge of #12803 |
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
2 participants
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
Continues the runner typing work from part 1 by introducing a
NodeTTypeVar onBaseRunner, then specializing each runner subclass to its concrete node type. This eliminates the remainingunion-attrandoverrideignores onself.nodeattribute accesses across the runner hierarchy.Commits, in order:
NodeTTypeVar toBaseRunner—BaseRunneris nowGeneric[NodeT, RunnerResultT];self.nodeis typed asNodeT;execute()takescompiled_node: NodeTCompileRunnerhierarchy —CompileRunner[CompilableNodeT],ModelRunner[ModelNode],FunctionRunner[FunctionNode],ShowRunner[ManifestSQLNode]; removes all 8union-attrignores inrun.pyand theoverrideignore onMicrobatchModelRunner.executeFreshnessRunner[SourceDefinition]and improveTestRunner— removesunion-attronself.node.source_name; correctsUnitTestDefinitionvsUnitTestNodetype usage inTestRunnerCloneRunnerandNoOpRunnerasBaseRunner[ResultNode, RunResult]freshness.py— retype_metadata_freshness_cachetoDict[BaseRelation, FreshnessResponse]; use a separate variable foradapter_response_dictto avoid reassignment ignoreTest plan
hatch run default:mypy core/dbt/passes (or ignore count decreases)hatch run default:pytest tests/unit/passes🤖 Generated with Claude Code