potiuk commented on code in PR #73181:
URL: https://github.com/apache/airflow/pull/73181#discussion_r4176506520
##########
providers/google/src/airflow/providers/google/cloud/hooks/bigtable.py:
##########
@@ -59,6 +61,28 @@ def __init__(
)
self._client: Client | None = None
+ def get_client_options(
+ self,
+ api_endpoint_override: str | None = None,
+ ) -> ClientOptions:
+ """
+ Return the ClientOptions object for Google Bigtable Admin API.
+
+ The returned options point at the Bigtable Admin API. ``Client``
passes the same options to its
+ data client, so data-plane methods added to this hook would need their
own endpoint.
+ """
+ if not self.is_default_universe():
+ if api_endpoint_override:
+ self.log.info(
+ "Ignoring api_endpoint_override because the universe
domain is not Google default universe."
+ )
+ global_universe_domain = os.getenv("GOOGLE_CLOUD_UNIVERSE_DOMAIN")
+ # google.cloud.bigtable.Client builds the admin channel from
api_endpoint only and ignores
+ # universe_domain, so the base hook's
ClientOptions(universe_domain=...) would still reach
+ # bigtableadmin.googleapis.com.
Review Comment:
Added a comment recording *why* the override exists — the Bigtable client
ignores `universe_domain` when it builds the admin channel — so it doesn't get
"simplified" away later.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
providers/google/tests/unit/google/cloud/hooks/test_bigtable.py:
##########
@@ -59,6 +59,32 @@ def setup_method(self):
):
self.bigtable_hook_no_default_project_id =
BigtableHook(gcp_conn_id="test")
+ @pytest.mark.parametrize("api_endpoint_override", [None,
"custom-override-api_endpoint"])
Review Comment:
Dropped the `is_default_universe` mock — it reads the same env var the test
already sets with `monkeypatch`, so the test now exercises the real path — and
parametrized it over with/without `api_endpoint_override` to cover the new log
line.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
providers/google/src/airflow/providers/google/cloud/hooks/bigtable.py:
##########
@@ -59,6 +61,28 @@ def __init__(
)
self._client: Client | None = None
+ def get_client_options(
+ self,
+ api_endpoint_override: str | None = None,
+ ) -> ClientOptions:
+ """
+ Return the ClientOptions object for Google Bigtable Admin API.
+
+ The returned options point at the Bigtable Admin API. ``Client``
passes the same options to its
+ data client, so data-plane methods added to this hook would need their
own endpoint.
+ """
+ if not self.is_default_universe():
+ if api_endpoint_override:
+ self.log.info(
+ "Ignoring api_endpoint_override because the universe
domain is not Google default universe."
Review Comment:
Outside the default universe `api_endpoint_override` was dropped silently,
while the base hook logs that it is ignoring it. Added the same log line for
consistency.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
providers/google/src/airflow/providers/google/cloud/hooks/bigtable.py:
##########
@@ -59,6 +61,28 @@ def __init__(
)
self._client: Client | None = None
+ def get_client_options(
+ self,
+ api_endpoint_override: str | None = None,
+ ) -> ClientOptions:
+ """
+ Return the ClientOptions object for Google Bigtable Admin API.
+
+ The returned options point at the Bigtable Admin API. ``Client``
passes the same options to its
+ data client, so data-plane methods added to this hook would need their
own endpoint.
Review Comment:
Noted in the docstring that these options target the Admin API. `Client`
passes them to the data client too, so data-plane methods added later would
need their own endpoint.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
