feat(wren-launcher): support MySQL and Postgres data source for dbt-tool #1876
Conversation
WalkthroughAdds MySQL data source support, changes Postgres port to string and adds DbName/SslDisable handling, updates profile parsing and tests, and augments the dbt README with required generated files and profile preparation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProfilesAnalyzer
participant Converter
participant DataSource
User->>ProfilesAnalyzer: Provide dbt profiles.yml
ProfilesAnalyzer->>Converter: Parsed DbtConnection (type, host, port, dbname, ssl_disable)
Converter->>DataSource: Determine datasource type
alt mysql
DataSource->>DataSource: convertToMysqlDataSource()
DataSource->>DataSource: Validate & MapType()
DataSource-->>Converter: WrenMysqlDataSource
else postgres
DataSource->>DataSource: convertToPostgresDataSource()
DataSource->>DataSource: Validate & MapType()
DataSource-->>Converter: WrenPostgresDataSource
end
Converter-->>User: Converted data source config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-07-09T02:43:20.433ZApplied to files:
🧬 Code Graph Analysis (1)wren-launcher/commands/dbt/data_source.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (11)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
wren-launcher/commands/dbt/data_source.go (1)
64-85: PostgreSQL conversion updates look good with one potential issue.The changes properly handle the DbName preference and port string conversion. However, there's a logical issue with port defaulting:
Issue: If
conn.Portis 0,strconv.Itoa(0)returns"0", not an empty string, so the default port logic won't trigger.Apply this fix:
- ds := &WrenPostgresDataSource{ - Host: conn.Host, - Port: strconv.Itoa(conn.Port), - Database: dbName, - User: conn.User, - Password: conn.Password, - } - - // If no port is specified, use PostgreSQL default port - if ds.Port == "" { - ds.Port = "5432" - } + port := strconv.Itoa(conn.Port) + if conn.Port == 0 { + port = "5432" + } + + ds := &WrenPostgresDataSource{ + Host: conn.Host, + Port: port, + Database: dbName, + User: conn.User, + Password: conn.Password, + }
🧹 Nitpick comments (1)
wren-launcher/commands/dbt/README.md (1)
1-13: Fix markdown formatting issues.The new documentation section is helpful and necessary, but there are formatting issues that should be addressed:
- ``` - dbt build - dbt docs generate - ``` + ```bash + dbt build + dbt docs generate + ```This fixes:
- Hard tabs replaced with spaces (lines 7-10)
- Added language specification to the code fence
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
wren-launcher/commands/dbt/README.md(1 hunks)wren-launcher/commands/dbt/converter.go(1 hunks)wren-launcher/commands/dbt/data_source.go(7 hunks)wren-launcher/commands/dbt/data_source_test.go(6 hunks)wren-launcher/commands/dbt/profiles.go(2 hunks)wren-launcher/commands/dbt/profiles_analyzer.go(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
Learnt from: narsik123
PR: Canner/WrenAI#1606
File: wren-ui/src/apollo/server/dataSource.ts:135-150
Timestamp: 2025-04-28T20:09:06.106Z
Learning: The Oracle data source in WrenAI includes an SSL toggle in the UI (OracleProperties component) and should pass this setting to the connection info using the pattern `...(ssl && { kwargs: { ssl: true } })` for consistency with other host-based connections like MS SQL.
📚 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/profiles_analyzer.gowren-launcher/commands/dbt/profiles.gowren-launcher/commands/dbt/data_source_test.gowren-launcher/commands/dbt/data_source.gowren-launcher/commands/dbt/converter.go
📚 Learning: 2025-04-28T20:09:06.106Z
Learnt from: narsik123
PR: Canner/WrenAI#1606
File: wren-ui/src/apollo/server/dataSource.ts:135-150
Timestamp: 2025-04-28T20:09:06.106Z
Learning: The Oracle data source in WrenAI includes an SSL toggle in the UI (OracleProperties component) and should pass this setting to the connection info using the pattern `...(ssl && { kwargs: { ssl: true } })` for consistency with other host-based connections like MS SQL.
Applied to files:
wren-launcher/commands/dbt/profiles_analyzer.gowren-launcher/commands/dbt/profiles.go
📚 Learning: 2025-07-09T02:43:07.560Z
Learnt from: goldmedal
PR: Canner/WrenAI#1827
File: wren-launcher/commands/dbt/data_source.go:154-156
Timestamp: 2025-07-09T02:43:07.560Z
Learning: In the WrenAI codebase, the MapType methods for data sources should return the original type name for unknown types, rather than a default fallback like "string". This maintains consistency across all data source implementations.
Applied to files:
wren-launcher/commands/dbt/converter.go
🧬 Code Graph Analysis (3)
wren-launcher/commands/dbt/data_source_test.go (2)
wren-launcher/commands/dbt/profiles.go (3)
DbtProfiles(4-7)DbtProfile(10-13)DbtConnection(16-42)wren-launcher/commands/dbt/data_source.go (3)
FromDbtProfiles(21-43)WrenPostgresDataSource(194-200)WrenMysqlDataSource(236-244)
wren-launcher/commands/dbt/data_source.go (1)
wren-launcher/commands/dbt/profiles.go (1)
DbtConnection(16-42)
wren-launcher/commands/dbt/converter.go (1)
wren-launcher/commands/dbt/data_source.go (1)
WrenMysqlDataSource(236-244)
🪛 markdownlint-cli2 (0.17.2)
wren-launcher/commands/dbt/README.md
7-7: Hard tabs
Column: 1
(MD010, no-hard-tabs)
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
8-8: Hard tabs
Column: 1
(MD010, no-hard-tabs)
9-9: Hard tabs
Column: 1
(MD010, no-hard-tabs)
10-10: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
wren-launcher/commands/dbt/profiles.go (2)
22-22: LGTM! Clean addition of PostgreSQL-specific database name field.The
DbNamefield addition is well-positioned and properly tagged, providing PostgreSQL-specific database naming support alongside the genericDatabasefield.
37-37: LGTM! Proper MySQL SSL configuration field.The
SslDisableboolean field is correctly implemented with appropriate YAML/JSON tags and clear commenting indicating its MySQL-specific purpose.wren-launcher/commands/dbt/converter.go (1)
157-168: LGTM! MySQL data source conversion properly implemented.The MySQL case follows the established pattern and correctly maps essential connection properties. The implementation aligns with the
WrenMysqlDataSourcestruct and includes SSL mode handling.wren-launcher/commands/dbt/profiles_analyzer.go (3)
130-130: LGTM! Proper PostgreSQL DbName field extraction.The extraction of the
dbnamefield follows the established pattern and uses the appropriate helper function.
144-144: LGTM! Correct MySQL SSL disable field extraction.The
ssl_disablefield extraction properly uses thegetBoolhelper function and includes clear database-specific commenting.
149-149: LGTM! Proper addition to known fields.Adding
dbnameto theknownFieldsmap prevents it from being treated as an unknown field, which is the correct behavior.wren-launcher/commands/dbt/data_source_test.go (4)
44-45: LGTM! Port field correctly updated to string type.The test expectation properly reflects the change from integer to string port representation in the
WrenPostgresDataSourcestruct.
68-127: LGTM! Excellent test coverage for PostgreSQL DbName field.The new test properly validates that PostgreSQL connections correctly handle the
DbNamefield and that the conversion logic prefersDbNameoverDatabaseas intended.
223-256: LGTM! PostgreSQL validation tests properly updated.All test cases correctly reflect the string port type and the invalid port test case appropriately uses an empty string to trigger validation failure.
269-324: LGTM! Comprehensive MySQL validation test coverage.The MySQL validation test follows the established testing pattern and covers all essential scenarios: valid configuration, missing host, missing database, and invalid port. This ensures the MySQL data source validation behaves correctly.
wren-launcher/commands/dbt/data_source.go (7)
6-6: Import addition looks good.The
strconvpackage import is necessary for the port conversion logic in both PostgreSQL and MySQL data source handling.
52-53: MySQL case addition is properly integrated.The new MySQL case follows the established pattern and correctly calls the dedicated conversion function.
196-196: PostgreSQL port field type change is correct.Changing from
inttostringaligns with the conversion logic and provides more flexibility for port handling.
218-227: PostgreSQL validation logic is comprehensive and correct.The port validation properly handles the new string type with appropriate format and range checks.
236-244: MySQL data source struct is well-designed.The struct includes all necessary fields with appropriate JSON tags and optional SSL configuration support.
246-273: MySQL interface methods are correctly implemented.Both
GetType()andValidate()methods follow established patterns and provide comprehensive validation.
275-311: Confirm TIMESTAMP → TIMESTAMPTZ mapping consistencyAlthough the MySQL type mappings in
WrenMysqlDataSource.MapTypeare comprehensive, our search only found this single instance ofTIMESTAMPTZin:
- wren-launcher/commands/dbt/data_source.go (MapType,
case "TIMESTAMP": return "TIMESTAMPTZ")No other references to
TIMESTAMPTZor related timestamp‐with‐timezone handling were detected in the codebase. Please verify that the wren‐engine integration expects MySQLTIMESTAMPcolumns to be mapped toTIMESTAMPTZ—otherwise this mapping may lead to unexpected behavior.
goldmedal
force-pushed
the
chore/dbt-mysql
branch
from
5631cee to
dd7eff9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
wren-launcher/commands/dbt/data_source.go (1)
75-89: LGTM: Unsupported types warn and continue.Matches the preference to proceed with nil, nil on unknown DB types.
🧹 Nitpick comments (6)
wren-launcher/commands/dbt/README.md (3)
7-10: Fix markdown lint: remove tabs and specify code fence language.Replace hard tabs and add a language for the code block to satisfy MD010/MD040.
- ``` - dbt build - dbt docs generate - ``` +```bash +dbt build +dbt docs generate +```
1-2: Polish heading and casing consistency.Use plural "Requirements" and consistent "dbt" casing per upstream docs.
-# Requirement for DBT project -This part outlines some requirements for the target dbt project: +# Requirements for dbt projects +This section outlines requirements for the target dbt project:
11-13: Consider adding minimal Postgres/MySQL profile examples to guide new users.A short profiles.yml snippet for Postgres (dbname) and MySQL (ssl_disable) will reduce setup friction. I can add it if you want.
wren-launcher/commands/dbt/data_source_test.go (2)
256-295: LGTM with a small addition: add a non-numeric port case.Validation already handles numeric parse. Consider adding an invalid non-numeric port case for completeness.
Example addition to invalidCases:
{ "non-numeric port", &WrenPostgresDataSource{ Host: testHost, Port: "abc", Database: "test", User: "user", }, },
297-352: LGTM: MySQL validation tests cover key cases. Add one more invalid port case.Add a "non-numeric port" case to ensure parse errors surface, and consider a conversion-path test to assert default port 3306 when conn.Port=0.
Apply within this test block:
@@ { name: "invalid port", ds: &WrenMysqlDataSource{ Host: "localhost", Port: "", Database: "test", User: "user", }, wantErr: true, }, + { + name: "non-numeric port", + ds: &WrenMysqlDataSource{ + Host: "localhost", + Port: "abc", + Database: "test", + User: "user", + }, + wantErr: true, + },Additionally, I can add:
- TestFromDbtProfiles_MySQL_DefaultPort: conn.Port=0 yields ds.Port "3306".
- TestFromDbtProfiles_Postgres_DefaultPort: conn.Port=0 yields ds.Port "5432".
Want me to push these tests?wren-launcher/commands/dbt/data_source.go (1)
306-342: MySQL type mapping may be inconsistent with canonical types; verify alignment with wren-engine.Return values mix lowercase ("char", "text") and uppercase ("INTEGER", "BIGINT", "TIMESTAMPTZ"). Ensure these match what Wren expects; otherwise normalize to a canonical set (e.g., integer, varchar, timestamp, double, boolean) like other mappers.
If normalization is desired, consider a table-driven map + lowercase outputs:
var mysqlToCanonical = map[string]string{ "CHAR": "char", "VARCHAR": "varchar", "TEXT": "text", "TINYTEXT": "text", "MEDIUMTEXT": "text", "LONGTEXT": "text", "ENUM": "text", "SET": "text", "BIT": "integer", "TINYINT": "integer", "SMALLINT": "integer", "MEDIUMINT": "integer", "INT": "integer", "INTEGER": "integer", "BIGINT": "integer", "FLOAT": "double", "DOUBLE": "double", "DECIMAL": "double", "NUMERIC": "double", "DATE": "date", "DATETIME": "timestamp", "TIMESTAMP": "timestamp", "BOOLEAN": "boolean", "BOOL": "boolean", "JSON": "json", }Happy to adjust to the exact wren-engine schema if you confirm the expected enum.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
wren-launcher/commands/dbt/README.md(1 hunks)wren-launcher/commands/dbt/converter.go(1 hunks)wren-launcher/commands/dbt/data_source.go(9 hunks)wren-launcher/commands/dbt/data_source_test.go(7 hunks)wren-launcher/commands/dbt/profiles.go(2 hunks)wren-launcher/commands/dbt/profiles_analyzer.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- wren-launcher/commands/dbt/converter.go
- wren-launcher/commands/dbt/profiles.go
🧰 Additional context used
🧠 Learnings (1)
📚 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/data_source_test.gowren-launcher/commands/dbt/profiles_analyzer.gowren-launcher/commands/dbt/data_source.go
🧬 Code Graph Analysis (2)
wren-launcher/commands/dbt/data_source_test.go (1)
wren-launcher/commands/dbt/data_source.go (2)
WrenPostgresDataSource(225-231)WrenMysqlDataSource(267-275)
wren-launcher/commands/dbt/data_source.go (1)
wren-launcher/commands/dbt/profiles.go (1)
DbtConnection(16-42)
🪛 markdownlint-cli2 (0.17.2)
wren-launcher/commands/dbt/README.md
7-7: Hard tabs
Column: 1
(MD010, no-hard-tabs)
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
8-8: Hard tabs
Column: 1
(MD010, no-hard-tabs)
9-9: Hard tabs
Column: 1
(MD010, no-hard-tabs)
10-10: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (6)
wren-launcher/commands/dbt/profiles_analyzer.go (2)
130-130: LGTM: dbname parsed for Postgres.
connection.DbName = getString("dbname")correctly supports Postgres’ dbname override.
144-144: LGTM: ssl_disable parsed for MySQL.
connection.SslDisable = getBool("ssl_disable")is appropriate for toggling SSL.wren-launcher/commands/dbt/data_source_test.go (2)
23-24: LGTM: Port assertions updated to string.Matches the data source refactor to string ports.
92-142: LGTM: Postgres DbName path covered.Good coverage asserting host/port/database/user/password and type.
wren-launcher/commands/dbt/data_source.go (2)
157-179: LGTM: MySQL conversion handles ssl_disable and default port correctly.Good logging, SSL mode derivation, and 3306 defaulting when conn.Port=0.
225-236: LGTM: Postgres port as string and type constant.Struct/tag updates and
GetTypeusing postgresType are consistent across codebase and tests.
Description
The type mapping tables follows the logic of wren eninge
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests