Conversation
📝 WalkthroughWalkthroughAdded runtime configuration for sqlparse grouping limits via new functions Sequence Diagram(s)sequenceDiagram
participant RuntimeInit as RuntimeInit
participant OutputMW as OutputMiddleware
participant SQLUtils as sql_utils.configure_sqlparse_limits
participant Sqlparse as sqlparse
participant Logger as Logger
RuntimeInit->>OutputMW: setup output middleware
RuntimeInit->>SQLUtils: call configure_sqlparse_limits()
SQLUtils->>Sqlparse: import sqlparse.engine.grouping
alt grouping API available
SQLUtils->>Sqlparse: set MAX_GROUPING_TOKENS / MAX_GROUPING_DEPTH
Sqlparse-->>SQLUtils: attributes updated
SQLUtils-->>RuntimeInit: return success
else API missing or error
SQLUtils->>Logger: log ImportError/AttributeError
SQLUtils-->>RuntimeInit: return (no-op)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Ruff (0.15.6)tests/unit/test_sql_utils.pyUnexpected Ruff issue shape at index 1 Comment |
|
📦 Python package built successfully!
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #77 +/- ##
==========================================
- Coverage 73.75% 73.71% -0.04%
==========================================
Files 93 93
Lines 5269 5284 +15
Branches 766 766
==========================================
+ Hits 3886 3895 +9
- Misses 1138 1144 +6
Partials 245 245
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/test_sql_utils.py`:
- Around line 13-50: The tests and fixture lack explicit return type
annotations; add return type hints to the autouse fixture
disable_sqlparse_limits (-> Iterator[None] or -> None depending on preferred
fixture typing) and to all test methods and the helper _build_large_select
(e.g., _build_large_select(num_columns: int = 5000) -> str, test_* -> None),
keeping existing signatures and imports intact; ensure you import typing symbols
if needed and do not change behavior of configure_sqlparse_limits,
reset_sqlparse_limits, is_single_select_query, or SQLParseError usages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7037270-7aaf-43f8-811e-d4fffeb5eeee
📒 Files selected for processing (3)
deepnote_toolkit/runtime_initialization.pydeepnote_toolkit/sql/sql_utils.pytests/unit/test_sql_utils.py
|
🚀 Review App Deployment Started
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/test_sql_utils.py`:
- Around line 13-14: The pytest fixture disable_sqlparse_limits should declare
its scope explicitly to signal per-test behavior; update the `@pytest.fixture`
decorator on the disable_sqlparse_limits fixture to include scope='function' so
intent is clear (i.e., change `@pytest.fixture` to
`@pytest.fixture`(scope='function')) while leaving the fixture body and
autouse=True unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dec8c247-a0a6-44e8-afac-e67f59d0ccc6
📒 Files selected for processing (1)
tests/unit/test_sql_utils.py
|
admin merging to release the hotfix asap will address dependency updates in a follow up PR to unblock CI / Audit checks |
Summary by CodeRabbit
New Features
Tests