feat(wren-launcher) support MSSQL data source for dbt-tools #1967
Conversation
WalkthroughAdds MSSQL support to the dbt conversion pipeline: new WrenMSSQLDataSource type with validation and type mapping, profiles model adds Server field, profiles analyzer parses server and improves port parsing, converter emits mssql WrenDataSource (container-aware host handling), and tests validate MSSQL profile conversion. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as Caller
participant PA as ProfilesAnalyzer
participant DS as DataSource Converter
participant C as Converter
participant O as Output
U->>PA: Provide dbt profiles (includes server/host, port, etc.)
PA->>PA: Parse connection (server/host, port parsing)
PA-->>DS: DbtConnection (type=sqlserver, fields)
DS->>DS: convertToMSSQLDataSource()
DS-->>C: WrenMSSQLDataSource
C->>C: Build WrenDataSource JSON (type="mssql")
alt Used by container
C->>C: handleLocalhostForContainer(host)
end
C-->>O: WrenDataSource {host, port, database, user, password, tds_version, driver, kwargs}
note over DS,C: New MSSQL path and properties population
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
wren-launcher/commands/dbt/profiles_analyzer.go (1)
109-114: Port parsing: handles int64 and numeric stringsNice robustness improvement. Consider trimming whitespace before Atoi to tolerate " 1433 ".
Example:
s := strings.TrimSpace(v) if i, err := strconv.Atoi(s); err == nil { return i }wren-launcher/commands/dbt/data_source_test.go (2)
223-288: Add a validation assertion to the MSSQL testTest covers fields well. Also assert Validate passes to catch future regressions.
if ds.Kwargs["TrustServerCertificate"] != "YES" { t.Errorf("Expected TrustServerCertificate 'YES', got '%s'", ds.Kwargs["TrustServerCertificate"]) } + if err := ds.Validate(); err != nil { + t.Errorf("Validation failed: %v", err) + } }
830-857: Deduplicate test utilitiesValidator and testDataSourceValidation are useful; consider moving to a shared test helper (e.g., data_source_test_helpers.go) to reuse across tests.
wren-launcher/commands/dbt/converter.go (1)
157-176: MSSQL JSON emission looks right; consider parity with MySQL for container host mappingHost remapping for containers mirrors Postgres. Apply the same pattern to MySQL for consistency when UsedByContainer = true.
wren-launcher/commands/dbt/data_source.go (1)
410-434: Password requirement may be too strictOther sources don’t require password in Validate. If integrated/AD auth is later supported, failing on empty password will block valid configs. Consider warning at connect-time or gating by auth mode instead of hard fail here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
wren-launcher/commands/dbt/converter.go(1 hunks)wren-launcher/commands/dbt/data_source.go(4 hunks)wren-launcher/commands/dbt/data_source_test.go(2 hunks)wren-launcher/commands/dbt/profiles.go(1 hunks)wren-launcher/commands/dbt/profiles_analyzer.go(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: goldmedal
PR: Canner/WrenAI#1887
File: wren-launcher/commands/dbt/data_source.go:273-276
Timestamp: 2025-08-13T05:17:30.180Z
Learning: In the Wren AI codebase, there's an intentional distinction between dbt connection types and Wren AI data source types: dbt uses "sqlserver" as the connection type, while Wren AI expects "mssql" as the data source type. The conversion layer in convertConnectionToDataSource() correctly matches "sqlserver" from dbt profiles and the WrenMSSQLDataSource.GetType() method correctly returns "mssql" for Wren AI compatibility.
📚 Learning: 2025-08-13T05:17:30.180Z
Learnt from: goldmedal
PR: Canner/WrenAI#1887
File: wren-launcher/commands/dbt/data_source.go:273-276
Timestamp: 2025-08-13T05:17:30.180Z
Learning: In the Wren AI codebase, there's an intentional distinction between dbt connection types and Wren AI data source types: dbt uses "sqlserver" as the connection type, while Wren AI expects "mssql" as the data source type. The conversion layer in convertConnectionToDataSource() correctly matches "sqlserver" from dbt profiles and the WrenMSSQLDataSource.GetType() method correctly returns "mssql" for Wren AI compatibility.
Applied to files:
wren-launcher/commands/dbt/profiles.gowren-launcher/commands/dbt/converter.gowren-launcher/commands/dbt/data_source.gowren-launcher/commands/dbt/data_source_test.go
📚 Learning: 2025-07-09T02:43:20.433Z
Learnt from: goldmedal
PR: Canner/WrenAI#1827
File: wren-launcher/commands/dbt/data_source.go:50-53
Timestamp: 2025-07-09T02:43:20.433Z
Learning: In wren-launcher/commands/dbt/data_source.go, when encountering unsupported database types in convertConnectionToDataSource function, the user prefers to log a warning and continue processing instead of failing. The function should return nil, nil to allow processing to continue.
Applied to files:
wren-launcher/commands/dbt/converter.gowren-launcher/commands/dbt/data_source.go
🧬 Code graph analysis (3)
wren-launcher/commands/dbt/converter.go (1)
wren-launcher/commands/dbt/data_source.go (1)
WrenMSSQLDataSource(395-404)
wren-launcher/commands/dbt/data_source.go (1)
wren-launcher/commands/dbt/profiles.go (1)
DbtConnection(16-44)
wren-launcher/commands/dbt/data_source_test.go (2)
wren-launcher/commands/dbt/profiles.go (3)
DbtProfiles(4-7)DbtProfile(10-13)DbtConnection(16-44)wren-launcher/commands/dbt/data_source.go (2)
FromDbtProfiles(63-85)WrenMSSQLDataSource(395-404)
🔇 Additional comments (7)
wren-launcher/commands/dbt/profiles.go (1)
19-19: MSSQL: clarify server vs host precedence and fallbackGood addition. Please document and enforce precedence (prefer Server for MSSQL, else fall back to Host). This avoids empty host when users put only host. See converter change below for host fallback.
wren-launcher/commands/dbt/profiles_analyzer.go (3)
8-8: Import of strconv is appropriateNeeded for flexible numeric parsing. LGTM.
153-153: Server extraction for MSSQLPopulating DbtConnection.Server is correct and aligns with dbt-sqlserver profiles.
160-160: Keep server in knownFieldsPrevents leakage into Additional. LGTM.
wren-launcher/commands/dbt/data_source.go (3)
17-33: Type constantsUseful consolidation for mappings. LGTM.
94-96: Recognize dbt 'sqlserver' and route to MSSQLMatches our convention (dbt: sqlserver → Wren: mssql). Good.
395-404: MSSQL data source shapeFields align with converter output; Kwargs allows extensibility. LGTM.
2 participants
Summary by CodeRabbit
New Features
Tests