feat: database version query#308
Conversation
WalkthroughA new method, Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dcs_core/core/datasource/sql_datasource.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
dcs_core/core/datasource/sql_datasource.py (2)
dcs_core/integrations/databases/mssql.py (1)
fetchone(107-108)dcs_core/integrations/databases/sybase.py (1)
fetchone(207-208)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Pytest Unit Tests with all supported python versions (3.12)
- GitHub Check: Pytest Unit Tests with all supported python versions (3.9)
| query = "SELECT @@version" | ||
| result = self.fetchone(query)[0] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coding SELECT @@version limits engine support
@@version is SQL-Server / Sybase-specific. SQLDataSource is dialect-agnostic, so subclasses pointing at Postgres, MySQL, etc. will break.
Consider:
- Inspecting the SQLAlchemy dialect (
self.connection.engine.name) and issuing the appropriate query (SELECT version()for Postgres/MySQL,PRAGMA user_versionfor SQLite, etc.). - Providing an overridable method in each concrete data-source subclass instead of implementing a one-size-fits-all query here.
🤖 Prompt for AI Agents
In dcs_core/core/datasource/sql_datasource.py around lines 182 to 183, the query
"SELECT @@version" is hard-coded, which only works for SQL Server and Sybase,
breaking compatibility with other databases. To fix this, detect the database
dialect using self.connection.engine.name and run the appropriate version query
for each dialect (e.g., "SELECT version()" for Postgres/MySQL, "PRAGMA
user_version" for SQLite). Alternatively, create an overridable method in
subclasses to provide the correct version query per database type, ensuring
dialect-agnostic behavior.
| query = "SELECT @@version" | ||
| result = self.fetchone(query)[0] | ||
|
|
||
| if result: | ||
| return result |
There was a problem hiding this comment.
Defensive-programming: guard against None rows and empty tuples
self.fetchone(query)[0] will raise TypeError if the query returns None, and IndexError if an empty tuple is returned. Fetch the row first, validate it, then unwrap.
- result = self.fetchone(query)[0]
-
- if result:
- return result
+ row = self.fetchone(query)
+ if row and len(row) > 0 and row[0]:
+ return row[0]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| query = "SELECT @@version" | |
| result = self.fetchone(query)[0] | |
| if result: | |
| return result | |
| query = "SELECT @@version" | |
| row = self.fetchone(query) | |
| if row and len(row) > 0 and row[0]: | |
| return row[0] |
🤖 Prompt for AI Agents
In dcs_core/core/datasource/sql_datasource.py around lines 182 to 186, the code
directly accesses the first element of the result from self.fetchone(query)
without checking if the result is None or an empty tuple, which can cause
TypeError or IndexError. Modify the code to first assign the result of
self.fetchone(query) to a variable, then check if this variable is not None and
contains at least one element before accessing the first element. Return the
first element only after these validations.
2 participants
User description
Fixes/Implements
Description
Summary Goes here.
Type of change
Delete irrelevant options.
How Has This Been Tested?
PR Type
Enhancement
Description
• Add database version query method to SQL datasource
• Implement version retrieval with fallback handling
Changes walkthrough 📝
sql_datasource.py
Add database version query functionalitydcs_core/core/datasource/sql_datasource.py
• Added
query_get_database_version()method• Implements SQL query
"SELECT @@Version" to retrieve database version
• Returns "Unknown
version" as fallback if no result found
Summary by CodeRabbit