test(intake): pin ClickHouse 26.3 LTS - #924
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughClickHouse is pinned to 26.3, propagated to local tooling and integration fixtures, documented as the supported LTS version, and validated for existing containers and server responses. Query comments now describe CTE behavior without version-specific wording. ChangesClickHouse version alignment
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
services/intake/tests/integration/spans/test_clickhouse_bootstrap.py (1)
14-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a concrete type hint for
run_async.The new test types the other parameters but leaves this fixture untyped. Use the repository’s concrete callable type, such as
Callable[[Any], Any], and import it normally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/intake/tests/integration/spans/test_clickhouse_bootstrap.py` around lines 14 - 18, Add a concrete callable type annotation to the run_async parameter in test_clickhouse_server_matches_supported_lts, using the repository’s established Callable[[Any], Any] form. Add the required Callable and Any imports normally without changing the test behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/intake/README.md`:
- Around line 63-64: Move the ClickHouse compatibility requirement from its
current position below run_clickhouse.sh to the prerequisites section near the
Docker prerequisite above the startup command. Keep the existing wording and
ensure users see the supported ClickHouse version before setup instructions.
---
Nitpick comments:
In `@services/intake/tests/integration/spans/test_clickhouse_bootstrap.py`:
- Around line 14-18: Add a concrete callable type annotation to the run_async
parameter in test_clickhouse_server_matches_supported_lts, using the
repository’s established Callable[[Any], Any] form. Add the required Callable
and Any imports normally without changing the test behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 88fd0241-0fd5-4623-b66f-c533cb6717cf
📒 Files selected for processing (6)
services/intake/.clickhouse-versionservices/intake/README.mdservices/intake/scripts/spans/run_clickhouse.shservices/intake/src/nmp/intake/spans/evaluation_session_repository.pyservices/intake/tests/integration/spans/conftest.pyservices/intake/tests/integration/spans/test_clickhouse_bootstrap.py
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
0454f7a to
8523cc8
Compare
|
Summary
Validation
ruff checkandruff format --checkty checkon changed Python files26.3image tagSummary by CodeRabbit
Bug Fixes
Tests
Documentation