Skip to content

Quick-win fixes: badge, exception type, is_integer, CI matrix, dead code#716

Draft
joshmarkovic wants to merge 5 commits into
dbt-msft:masterfrom
joshmarkovic:fix/audit-quick-wins-2
Draft

Quick-win fixes: badge, exception type, is_integer, CI matrix, dead code#716
joshmarkovic wants to merge 5 commits into
dbt-msft:masterfrom
joshmarkovic:fix/audit-quick-wins-2

Conversation

@joshmarkovic

@joshmarkovic joshmarkovic commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Five small, independent fixes surfaced by a repo audit. Each is its own commit, so they can be reviewed individually. They are intentionally removal-heavy and low-risk: the only behavioral change is #2, whose existing unit test is updated to match.

Branch validation (whole branch, on top of current master): ruff clean, black --check clean, mypy reports 0 errors, and pytest tests/unit → 110 passed.

1. Remove broken "Integration tests on Azure" badge — docs

README.md rendered a status badge pointing at integration-tests-azure.yml, but no such workflow exists under .github/workflows/ (only unit-tests, integration-tests-sqlserver, publish-docker, and release-version). The badge was therefore permanently broken. Removed that one badge line; the unit-test and SQL Server integration badges are unchanged.

2. Raise DbtRuntimeError instead of bare ValueError for a missing access token — fix

In sqlserver_auth.py, the ActiveDirectoryAccessToken path raised a bare ValueError when access_token / access_token_expires_on were missing. Every other configuration fault in this layer raises dbt_common.exceptions.DbtRuntimeError, so this one escaped as an unclassified exception. It now raises DbtRuntimeError (message unchanged). The existing unit test that asserted ValueError is updated to expect DbtRuntimeError.

3. Recognize tinyint in is_integer(); drop dead Postgres aliases — fix

SQLServerColumn.is_integer() listed smallserial, serial, bigserial, int2/4/8, and serial2/4/8 — Postgres types that SQL Server never returns — while omitting tinyint, a real SQL Server integer type. Replaced the list with the actual SQL Server integer set: tinyint, smallint, int, integer, bigint.

4. Drop Python 3.9 from the publish-docker matrix — ci

requires-python is >=3.10, and the unit / integration / release workflows all target 3.10–3.13, but publish-docker.yml still built CI-3.9-* client images. Those images are not consumed by any workflow. Removed "3.9" from the matrix.

5. Remove dead clone macro and unused view scaffolding variable — chore

  • materializations/models/table/clone.sql defined sqlserver__create_or_replace_clone, which emits invalid T-SQL (CREATE TABLE … AS CLONE OF …). It is unreachable: sqlserver__can_clone_table() returns False, so dbt-core's clone materialization never dispatches to it, and default__create_or_replace_clone exists as the fallback regardless. Deleted the file. sqlserver__can_clone_table() (in relations/table/clone.sql) is left untouched.
  • relations/views/create.sql contained a {% set tst %}SELECT '1' as col{% endset %} block that was never referenced. Removed.

The integration-tests-azure.yml workflow does not exist, so the badge always rendered broken.
…oken

ActiveDirectoryAccessToken config faults now surface as a dbt error, consistent with the other auth/config validations in this layer. Updates the unit test that asserted the old type.
…iases

SQL Server integer types are tinyint/smallint/int/bigint; the serial and int2/4/8 aliases were Postgres copy-paste and are never returned by SQL Server.
requires-python is >=3.10; the 3.9 images were unused by the unit/integration/release workflows.
sqlserver__create_or_replace_clone was unreachable (sqlserver__can_clone_table returns False and dbt-core provides default__create_or_replace_clone) and emitted invalid T-SQL. The unused 'tst' set block in create_view_as is also removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant