Skip to content

[TIDY-FIRST] Type the runners - Part 3#12811

Merged
QMalcolm merged 2 commits intomainfrom
qmalcolm--tidy-first-type-the-runners-part-3
Apr 10, 2026
Merged

[TIDY-FIRST] Type the runners - Part 3#12811
QMalcolm merged 2 commits intomainfrom
qmalcolm--tidy-first-type-the-runners-part-3

Conversation

@QMalcolm
Copy link
Copy Markdown
Contributor

@QMalcolm QMalcolm commented Apr 9, 2026

Summary

  • GenericSqlRunner's result type (RemoteCompileResultMixin) lives in a separate hierarchy from NodeResult/RunResult, so it cannot satisfy the RunnerResultT bound on BaseRunner.
  • Makes GenericSqlRunner a standalone Generic[SQLResult] class with its own __init__, compile, safe_run, and exception handling — eliminating all 11 # type: ignore comments in sql.py.
  • Also removes dead code in handle_exception: dbt.exceptions.Exception does not exist, making the isinstance branch an AttributeError at runtime. A REVIEW comment already flagged this; removing it now that the method has explicit type annotations that surface the issue.

Test plan

  • hatch run default:mypy core/dbt/ passes (or ignore count decreases)
  • hatch run default:pytest tests/unit/ passes
  • SQL runner functional tests pass

🤖 Generated with Claude Code

@QMalcolm QMalcolm requested a review from a team as a code owner April 9, 2026 19:36
@cla-bot cla-bot bot added the cla:yes label Apr 9, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

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 QMalcolm added the Skip Changelog Skips GHA to check for changelog file label Apr 9, 2026
@QMalcolm QMalcolm changed the title [TIDY-FIRST] Type the runners, part 3: Decouple GenericSqlRunner from BaseRunner [TIDY-FIRST] Type the runners - Part 3 Apr 9, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 0% with 60 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.24%. Comparing base (43defc9) to head (d1b67b0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12811      +/-   ##
==========================================
- Coverage   91.41%   91.24%   -0.17%     
==========================================
  Files         203      203              
  Lines       25881    25916      +35     
==========================================
- Hits        23659    23648      -11     
- Misses       2222     2268      +46     
Flag Coverage Δ
integration 88.04% <0.00%> (-0.23%) ⬇️
unit 65.45% <0.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Unit Tests 65.45% <0.00%> (-0.09%) ⬇️
Integration Tests 88.04% <0.00%> (-0.23%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@QMalcolm QMalcolm removed the Skip Changelog Skips GHA to check for changelog file label Apr 9, 2026
@QMalcolm
Copy link
Copy Markdown
Contributor Author

QMalcolm commented Apr 9, 2026

@QMalcolm QMalcolm force-pushed the qmalcolm--tidy-first-type-the-runners-part-2 branch from 122b135 to 6c831bb Compare April 9, 2026 20:07
@QMalcolm QMalcolm force-pushed the qmalcolm--tidy-first-type-the-runners-part-3 branch from 388b914 to 2ef1329 Compare April 9, 2026 20:08
MichelleArk
MichelleArk previously approved these changes Apr 10, 2026
@QMalcolm QMalcolm force-pushed the qmalcolm--tidy-first-type-the-runners-part-2 branch from 6c831bb to 96eb75e Compare April 10, 2026 15:43
Base automatically changed from qmalcolm--tidy-first-type-the-runners-part-2 to main April 10, 2026 17:14
@QMalcolm QMalcolm dismissed MichelleArk’s stale review April 10, 2026 17:14

The base branch was changed.

QMalcolm and others added 2 commits April 10, 2026 12:28
GenericSqlRunner's result type (RemoteCompileResultMixin) lives in a
separate hierarchy from NodeResult/RunResult, so it cannot satisfy the
RunnerResultT bound on BaseRunner. Make GenericSqlRunner a standalone
Generic[SQLResult] class with its own __init__, compile, safe_run, and
exception handling — eliminating all 11 # type: ignore comments in sql.py.

Also removes dead code in handle_exception: dbt.exceptions.Exception
does not exist, making the isinstance branch an AttributeError at runtime.
The REVIEW comment already flagged this; removing it now that the method
has explicit type annotations that surface the issue.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@QMalcolm QMalcolm force-pushed the qmalcolm--tidy-first-type-the-runners-part-3 branch from 2ef1329 to d1b67b0 Compare April 10, 2026 17:28
@QMalcolm
Copy link
Copy Markdown
Contributor Author

Rebased off main since #12803 and #12810 were squash merged

@QMalcolm QMalcolm merged commit 9467792 into main Apr 10, 2026
122 checks passed
@QMalcolm QMalcolm deleted the qmalcolm--tidy-first-type-the-runners-part-3 branch April 10, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants