| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -47,6 +47,7 @@ | |
| NotSupportedError, | ||
| ProgrammingError, | ||
| ) | ||
| from databricks.sql.telemetry.telemetry_client import TelemetryHelper | ||
| from databricks.sql.thrift_api.TCLIService import ttypes | ||
|
|
||
| if TYPE_CHECKING: | ||
| Expand Down Expand Up | @@ -165,6 +166,31 @@ def _is_staging_statement(operation: str) -> bool: | |
| return verb in _STAGING_VERBS | ||
|
|
||
|
|
||
| def _kernel_telemetry_kwargs(options: Dict[str, Any]) -> Dict[str, Any]: | ||
| """Build phase-7 telemetry/system kwargs for ``databricks_sql_kernel.Session``.""" | ||
| system = TelemetryHelper.get_driver_system_configuration() | ||
| out: Dict[str, Any] = { | ||
| "driver_name": system.driver_name, | ||
| "driver_version": system.driver_version, | ||
| "runtime_name": system.runtime_name, | ||
| "runtime_version": system.runtime_version, | ||
| "runtime_vendor": system.runtime_vendor, | ||
| "os_name": system.os_name, | ||
| "os_version": system.os_version, | ||
| "os_arch": system.os_arch, | ||
| "client_app_name": system.client_app_name, | ||
| "locale_name": system.locale_name, | ||
| "char_set_encoding": system.char_set_encoding, | ||
| # The Python telemetry model does not currently track process | ||
| # name; omit it and let the kernel fill what it can derive. | ||
| "process_name": None, | ||
| "telemetry_enabled": bool(options.get("enable_telemetry", True)), | ||
| } | ||
| if options.get("telemetry_batch_size") is not None: | ||
| out["telemetry_batch_size"] = options["telemetry_batch_size"] | ||
| return out | ||
|
|
||
|
|
||
| # ─── Client ───────────────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| Expand Down Expand Up | @@ -217,6 +243,9 @@ def __init__( | |
| # to the kernel ``Session``'s ``retry_*`` kwargs in | ||
| # ``open_session`` via ``_kernel_retry_kwargs``. | ||
| self._retry_options = kwargs.get("retry_options") or {} | ||
| # Kernel telemetry phase 7 adds binding/runtime identity and | ||
| # telemetry config kwargs directly to ``databricks_sql_kernel.Session``. | ||
| self._telemetry_options = kwargs.get("telemetry_options") or {} | ||
| self._catalog = catalog | ||
| self._schema = schema | ||
| # ``_use_arrow_native_complex_types`` is the connector-side | ||
| Expand Down Expand Up | @@ -316,6 +345,7 @@ def open_session( | |
| # Translate the connector's ``_retry_*`` kwargs into the | ||
| # kernel's ``retry_*`` kwargs. Empty when at defaults. | ||
| retry_kwargs = _kernel_retry_kwargs(self._retry_options) | ||
| telemetry_kwargs = _kernel_telemetry_kwargs(self._telemetry_options) | ||
| # Forward caller / connector HTTP headers. The kernel applies | ||
| # them on every request; a caller ``User-Agent`` is appended | ||
| # to the kernel's base UA. Only pass the kwarg when there's | ||
| Expand Down Expand Up | @@ -358,6 +388,7 @@ def open_session( | |
| **auth_kwargs, | ||
| **tls_kwargs, | ||
| **retry_kwargs, | ||
| **telemetry_kwargs, | ||
|
Comment thread
Copy link
Copy Markdown
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🔵 Low — Unlike retry_kwargs/http_headers_kwargs (which are conditionally omitted when empty), _kernel_telemetry_kwargs always returns the phase-7 identity fields (driver_name, runtime_*, os_*, process_name, telemetry_enabled), so these kwargs are passed to _kernel.Session(...) on every use_kernel open. If the installed kernel wheel predates phase-7 support for these Session kwargs, construction raises TypeError and every use_kernel=True connection breaks. The dependency floor is still ^0.2.0 and isn't bumped in this PR. If phase-7 requires a newer kernel wheel, consider raising the minimum version so incompatible wheels fail at install time rather than at connect time. (Low because I can't verify the 0.2.0 Session signature from this repo — the kernel is a compiled extension.)
Sorry, something went wrong.
All reactions
|
||
| **http_headers_kwargs, | ||
| ) | ||
| except Exception as exc: | ||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -122,6 +122,9 @@ def get_auth_flow(auth_provider): | |
|
|
||
| @staticmethod | ||
| def is_telemetry_enabled(connection: "Connection") -> bool: | ||
| if getattr(connection.session, "use_kernel", False) is True: | ||
|
Comment thread
Copy link
Copy Markdown
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🔵 Low — The kernel bypass uses an identity check (is True), but backend routing in _create_backend uses truthiness (if self.use_kernel: where self.use_kernel = kwargs.get("use_kernel", False)). These disagree for any truthy-but-non-True value: e.g. use_kernel=1 or use_kernel="true" would still route the connection through KernelDatabricksClient (truthy), yet 1 is True / "true" is True evaluate to False, so Python-side telemetry would NOT be disabled — defeating the intent of this PR for those inputs. Recommend matching the routing semantics with a plain truthiness check so the two code paths can't diverge: if getattr(connection.session, "use_kernel", False):
return FalseMinor, since use_kernel is documented/expected to be a bool, but the mismatch is a latent inconsistency.
Sorry, something went wrong.
All reactions
|
||
| return False | ||
|
|
||
| # Fast path: force enabled - skip feature flag fetch entirely | ||
| if connection.force_enable_telemetry: | ||
| return True | ||
| Expand Down | ||
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🔵 Low — _kernel_telemetry_kwargs builds the phase-7 identity/telemetry kwargs (driver_name, telemetry_enabled, process_name, etc.) and they are spread unconditionally into _kernel.Session(**telemetry_kwargs) at open_session. The kernel wheel constraint is still ^0.2.0 (>=0.2.0,<0.3.0). If these kwargs were introduced in a later 0.2.x than 0.2.0, a user with an older-but-constraint-satisfying wheel installed would hit a TypeError: Session() got an unexpected keyword argument ... at connect time. If phase-7 requires a minimum kernel version, consider bumping the lower bound of the databricks-sql-kernel pin so the wheel and connector stay in lockstep. (Flagged Low — I can't verify the kernel Session signature from this repo.)
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.