jayamanikharyono commented on code in PR #72768:
URL: https://github.com/apache/airflow/pull/72768#discussion_r4307507266
##########
providers/clickhousedb/src/airflow/providers/clickhousedb/hooks/clickhouse.py:
##########
@@ -236,6 +243,9 @@ def _get_client_kwargs(self) -> dict[str, Any]:
k: v for k, v in merged_client_kwargs.items() if k not in
_HOOK_MANAGED_KWARGS
}
+ # Shared/reused client -> no auto-generated session ID unless the
caller opts in.
+ kwargs.setdefault("autogenerate_session_id", False)
Review Comment:
Thanks for catching this! I've implemented your first suggestion: a session
per `get_conn()` caller.
The hook now gives each `get_conn()` connection its own ClickHouse session
on the shared client, by sending a `session_id` with every query instead of
setting it on the client. All statements in one `run()` /
`SQLExecuteQueryOperator` task share that session, so your temp-table and `SET`
examples work as before, and connections in different threads still get
separate sessions.
One behaviour change remains: `get_client()` now returns a client without a
session, and `bulk_insert_rows()` uses that same client. I noted this in
`changelog.rst`, and users can pass `client_kwargs={"autogenerate_session_id":
True}` to give it a session again (shared across the whole hook, so not
thread-safe). If users configure sessions themselves (via `client_kwargs`,
`session_settings`, or clickhouse-connect's global setting), the hook leaves
them alone.
I've also:
- Added tests checking that a multi-statement `run()` shares one session
(based on your temp-table example), that each `run()` gets a fresh session, and
that the hook respects user-configured sessions.
- Added a "Multi-statement SQL and sessions" section to the operator docs.
- Updated the PR description, since "No public API change" wasn't accurate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
