[client-v2] Session API with reusable Session objects#2810
[client-v2] Session API with reusable Session objects#2810
Conversation
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
There was a problem hiding this comment.
Pull request overview
Implements first-class session support in the client-v2 API, allowing users to set ClickHouse session_id, session_check, and session_timeout at both client and per-operation levels.
Changes:
- Adds session query-parameter constants and propagates session settings through common/query/insert/command settings.
- Extends
Client.BuilderandClientwith session setters and runtime update methods. - Adds unit and integration tests verifying session parameter serialization and precedence (client-level vs operation-level).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client-v2/src/main/java/com/clickhouse/client/api/http/ClickHouseHttpProto.java | Adds HTTP query parameter constants for session settings. |
| client-v2/src/main/java/com/clickhouse/client/api/internal/CommonSettings.java | Implements session setting storage/validation in shared settings layer. |
| client-v2/src/main/java/com/clickhouse/client/api/query/QuerySettings.java | Exposes session setters/getters for query operations. |
| client-v2/src/main/java/com/clickhouse/client/api/insert/InsertSettings.java | Exposes session setters/getters for insert operations. |
| client-v2/src/main/java/com/clickhouse/client/api/command/CommandSettings.java | Provides covariant fluent overrides for new session setters. |
| client-v2/src/main/java/com/clickhouse/client/api/Client.java | Adds builder-level session configuration and runtime update APIs. |
| client-v2/src/test/java/com/clickhouse/client/SettingsTests.java | Adds unit tests for session settings validation and accessors. |
| client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java | Adds integration tests verifying session params in outgoing HTTP requests and runtime updates. |
client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java
Outdated
Show resolved
Hide resolved
client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java
Outdated
Show resolved
Hide resolved
|
@claude[agent] review changes do not fix. |
Could you clarify what you'd like me to do? Should I:
The current code has synchronization added to address the thread-safety concern from the review. |
|
@claude[agent] review new session API |
I've completed a comprehensive review of the session API implementation. Here are my findings: ✅ Implementation QualityStrengths:
|
|
@claude[agent] review latest changes but do not change code. |
I've reviewed the latest changes. Here's my analysis: 🎯 Major Improvements (commits 1d68550 & 6f1ca06)1. Session Object Introduction ✅
2. Architecture Improvements ✅Client refactoring (Client.java:149-151):
Benefits:
3. API Enhancements ✅Builder pattern:
Benefits:
4. Documentation & Examples ✅examples/client-v2/Sessions.java:
docs/features.md updates:
5. Test Coverage ✅New tests:
Test improvements:
📋 Code Quality Assessment✅ Strengths
|
|



Summary
Implements first-class session support in the client-v2 API with a reusable
Sessionconfiguration object. Allows users to configure ClickHouse HTTP sessions (session_id,session_check,session_timeout,session_timezone) at both client and per-operation levels.Closes #2345
Checklist
Delete items not relevant to your PR:
Changes Made
Core API
Sessionclass: Thread-safe, reusable configuration object encapsulating all session settings.use(Session)pattern: Consistent API acrossClient.Builderand all settings classes (QuerySettings,InsertSettings,CommandSettings) for applying session configurationQPARAM_SESSION_ID,QPARAM_SESSION_CHECK,QPARAM_SESSION_TIMEOUT,QPARAM_SESSION_TIMEZONEtoClickHouseHttpProtoClient.Builder.use(Session)for client-wide session defaultsClient.Builder.setSessionId/Check/Timeout/Timezone()for individual session propertiesClient.updateSessionId()for runtime updates (onlysession_idis mutable; other session properties are fixed after client construction)CommonSettings.use(Session)propagates toQuerySettings,InsertSettings, andCommandSettingsArchitecture
Collections.synchronizedMap()withConcurrentHashMapfor better concurrent performanceSessionfield withSession.applyTo()method for request buildingSessionclass, general configuration inClient.configurationDocumentation & Examples
Testing
session_idupdates.use(Session)pattern functionalityCompatibility
Sessionclass with all session configuration methodsClient.Builder.use(Session)and individual session settersClient.updateSessionId()for runtime updatesCommonSettings.use(Session)and subclassesClickHouseHttpProto.QPARAM_SESSION_*constantssession_idis mutable at runtime