Skip to content

Add optional dbname override to MicrosoftSQLServerConnector.build_uri#8017

Merged
adamsachs merged 3 commits into
mainfrom
fix/mssql-connector-build-uri-for-database
Apr 29, 2026
Merged

Add optional dbname override to MicrosoftSQLServerConnector.build_uri#8017
adamsachs merged 3 commits into
mainfrom
fix/mssql-connector-build-uri-for-database

Conversation

@adamsachs
Copy link
Copy Markdown
Contributor

@adamsachs adamsachs commented Apr 23, 2026

https://ethyca.atlassian.net/browse/ENG-3592

Description Of Changes

The fidesplus MSSQL discovery monitor needs to target a specific database on each scan iteration. Today it duplicates the URL-construction logic from MicrosoftSQLServerConnector.build_uri (in its own create_engine method) because there's no way to parameterize the dbname through the connector. That duplication is how read_only_connection ended up silently ignored by D&D scans — see fidesplus#3459.

This PR adds an optional dbname: Optional[str] = None parameter to MicrosoftSQLServerConnector.build_uri. When provided, it overrides the database name from secrets; otherwise behavior is unchanged. The companion fidesplus PR (stacked on 3459) will swap the monitor's inline URL construction for connector.build_uri(db_name).

Code Changes

  • src/fides/api/service/connectors/microsoft_sql_server_connector.pybuild_uri now accepts an optional dbname argument; URL.create(..., database=dbname or config.dbname, ...). No other behavior changes.
  • tests/ops/service/connection_config/test_microsoft_sql_server_connector.py — New test module covering default behavior, dbname override (including assertion that secrets are not mutated), and read_only_connection routing across enabled/disabled/unset cases (parameterized).

Steps to Confirm

  1. Run pytest tests/ops/service/connection_config/test_microsoft_sql_server_connector.py — all 5 tests (1 default + 1 override + 3 read_only param cases) should pass.
  2. No behavior change for existing callers: connector.build_uri() without arguments is unchanged.

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • No new entry — tiny method-signature addition; will ship via the paired fidesplus fix entry
  • UX feedback:
    • No UX review needed
  • Followup issues:
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

🤖 Generated with Claude Code

The MSSQL discovery monitor in fidesplus needs to target a different
database per scan without mutating the connection config's secrets.
Extending build_uri with an optional dbname override is cheaper than
duplicating URL-construction logic (which is how read_only_connection
ended up unsupported in the monitor — see fidesplus#3459).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Apr 28, 2026 2:49pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Apr 28, 2026 2:49pm

Request Review

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.96%. Comparing base (353115b) to head (2381bcb).
⚠️ Report is 17 commits behind head on main.

❌ Your project status has failed because the head coverage (84.96%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8017   +/-   ##
=======================================
  Coverage   84.96%   84.96%           
=======================================
  Files         632      632           
  Lines       41471    41471           
  Branches     4836     4836           
=======================================
+ Hits        35235    35237    +2     
+ Misses       5136     5134    -2     
  Partials     1100     1100           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

adamsachs and others added 2 commits April 23, 2026 10:24
@adamsachs adamsachs marked this pull request as ready for review April 28, 2026 16:48
@adamsachs adamsachs requested a review from a team as a code owner April 28, 2026 16:48
@adamsachs adamsachs requested review from dsill-ethyca and galvana and removed request for a team and galvana April 28, 2026 16:48
@adamsachs adamsachs added this pull request to the merge queue Apr 29, 2026
Merged via the queue into main with commit d696b37 Apr 29, 2026
70 of 71 checks passed
@adamsachs adamsachs deleted the fix/mssql-connector-build-uri-for-database branch April 29, 2026 19:25
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