ctk settings: Workaround for SQLAlchemy sslmode error#531
Conversation
WalkthroughAdds native CrateDB HTTP API support for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant DB as util.database
participant CrateClient as crate_client
participant SQLA as SQLAlchemy
participant HTTP as CrateDB HTTP _sql
Caller->>DB: run_sql_real(dburi, statements, params, records)
alt crate:// and native=true
Note over DB: [Native] path — crate_client + HTTP
DB->>CrateClient: connect(dburi_clean, ssl_verify=False)
loop for each statement
DB->>HTTP: POST native_url/_sql { stmt, args }
HTTP-->>DB: { cols, rows, ... } or error
alt records == True
Note right of DB: map rows -> dicts using cols
else
Note right of DB: return raw rows
end
end
else non-native / other schemes
Note over DB: [SQLAlchemy] path
DB->>SQLA: create_engine(dburi) / execute(stmt, params)
loop for each statement
SQLA-->>DB: results
end
end
alt single statement
DB-->>Caller: single result
else multiple statements
DB-->>Caller: list of results
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)cratedb_toolkit/util/database.py (2)
🪛 Ruff (0.12.2)cratedb_toolkit/util/database.py162-162: Probable use of (S113) 164-164: Probable use of (S501) 🪛 GitHub Actions: Tests: Commoncratedb_toolkit/util/database.py[error] 162-162: Ruff check: S113 Probable use of [error] 164-164: Ruff check: S501 Probable use of ⏰ 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). (6)
🔇 Additional comments (2)
✨ 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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
cratedb_toolkit/util/database.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cratedb_toolkit/util/database.py (2)
cratedb_toolkit/model.py (4)
dburi(103-107)DatabaseAddress(55-197)from_string(63-72)from_string(223-224)cratedb_toolkit/adapter/rockset/server/api/query.py (1)
execute(36-68)
🪛 Ruff (0.12.2)
cratedb_toolkit/util/database.py
162-162: Probable use of requests call without timeout
(S113)
164-164: Probable use of requests call with verify=False disabling SSL certificate checks
(S501)
⏰ 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). (8)
- GitHub Check: build-and-publish
- GitHub Check: build-and-test
- GitHub Check: Generic: Python 3.13 on OS ubuntu-latest
- GitHub Check: Generic: Python 3.9 on OS ubuntu-latest
- GitHub Check: CFR for OS windows-latest
- GitHub Check: CFR for OS macos-13
- GitHub Check: CFR for OS macos-latest
- GitHub Check: CFR for OS ubuntu-latest
🔇 Additional comments (2)
cratedb_toolkit/util/database.py (2)
176-186: SQLAlchemy path looks fine.Context manager is applied; result shaping mirrors prior behavior.
189-189: Single-item unwrapping is safe: downstream callers either index into the returned data or ignore the return value, and existing tests (e.g. tests/util/database.py) already expect[(1,)]rather than a nested list. No further changes needed.
amotl
force-pushed
the
ta/settings-compare-workaround
branch
from
ad2f1a6 to
b90e194
Compare
|
Hi @tomach. Do you need this change? Please let us know so we may be able to help accordingly. 🙏 |
|
Hi @amotl,
Not really. this was just a temporary (and admittedly ugly 😅) workaround I shared with @plaharanne, who was also hitting the same I don’t think this should be merged as-is. I didn’t have time to investigate the root cause yet... |
|
Thanks. Maybe we should converge your error report into a dedicated issue if it's still applicable? Something like this might be needed, or something different in the same area? |
2 participants
About
A workaround for a yet unknown bug submitted by @tomach last week. Is it applicable or can we find a different one?