Skip to content

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

Merged
axellpadilla merged 4 commits into
dbt-msft:masterfrom
joshmarkovic:fix/audit-quick-wins-2
Jun 22, 2026
Merged

Quick-win fixes: badge, exception type, is_integer, CI matrix, dead code#716
axellpadilla merged 4 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

Four 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.

An earlier is_integer() cleanup (add tinyint, drop dead Postgres aliases) was dropped from this PR. #702 already adds tinyint to is_integer(), so that work belongs there to avoid a conflict. See the note on #702 about also dropping the dead aliases.

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. 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 to 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.

4. 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.
@joshmarkovic joshmarkovic marked this pull request as ready for review June 22, 2026 01:29
@axellpadilla

Copy link
Copy Markdown
Collaborator

@joshmarkovic you will have conflicts with #702 I suppose

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.
@joshmarkovic

Copy link
Copy Markdown
Contributor Author

Thanks @axellpadilla, good catch. The only real overlap was is_integer() in sqlserver_column.py, which #702 already covers (it adds tinyint). I've dropped that commit, so #716 is now four independent fixes (badge, exception type, publish-docker py3.9 matrix, and dead-code removal), none of which touch any file in #702. No conflict anymore.

One note for #702 while it's in that file: it keeps the dead Postgres aliases (smallserial/serial/bigserial/int2/int4/int8/serial2/serial4/serial8) that SQL Server never returns. Left a line over there.

@axellpadilla axellpadilla merged commit d4ecf7e into dbt-msft:master Jun 22, 2026
16 checks passed
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.

2 participants