github-actions[bot] commented on code in PR #40674:
URL: https://github.com/apache/beam/pull/40674#discussion_r4320951998
##########
sdks/python/apache_beam/io/gcp/gcsio_test.py:
##########
@@ -959,6 +959,52 @@ def test_is_soft_delete_enabled(self, mock_get_bucket):
self.gcs.is_soft_delete_enabled("gs://beam_without_soft_delete/tmp"))
+_TPC_UNIVERSE_DOMAIN = "example-tpc.goog"
+
+
[email protected](NotFound is None, 'GCP dependencies are not installed')
+# Disable mTLS auto-detection, which is incompatible with non-default
+# universe domains and may be enabled on developer machines.
[email protected](os.environ, {"GOOGLE_API_USE_CLIENT_CERTIFICATE": "false"})
+class TestCreateStorageClientUniverseDomain(unittest.TestCase):
+ """Tests for Trusted Partner Cloud (TPC) universe domain support."""
+ def _create_client(self, mock_get_service_credentials, universe_domain):
+ from apache_beam.internal.gcp.auth import _ApitoolsCredentialsAdapter
+ mock_get_service_credentials.return_value = _ApitoolsCredentialsAdapter(
+ _make_credentials("test-project", universe_domain=universe_domain))
+ return gcsio.create_storage_client(
+ PipelineOptions(["--project=test-project"]))
+
+ @mock.patch('apache_beam.internal.gcp.auth.get_service_credentials')
+ def test_default_universe_domain(self, mock_get_service_credentials):
+ os.environ.pop("GOOGLE_CLOUD_UNIVERSE_DOMAIN", None)
+ client = self._create_client(
+ mock_get_service_credentials, _DEFAULT_UNIVERSE_DOMAIN)
+ self.assertEqual(client.universe_domain, _DEFAULT_UNIVERSE_DOMAIN)
+ self.assertEqual(
+ client._connection.API_BASE_URL, "https://storage.googleapis.com";)
+
+ @mock.patch('apache_beam.internal.gcp.auth.get_service_credentials')
+ def test_universe_domain_from_env_var(self, mock_get_service_credentials):
+ os.environ["GOOGLE_CLOUD_UNIVERSE_DOMAIN"] = _TPC_UNIVERSE_DOMAIN
Review Comment:
**🟡 Medium**: `test_universe_domain_from_env_var` mutates
`os.environ["GOOGLE_CLOUD_UNIVERSE_DOMAIN"]` directly, and the class-level
`@mock.patch.dict(os.environ, ...)` only snapshots/restores `os.environ` at the
method level if applied to the test method or if `clear=False` is used
per-method (when decorating a `unittest.TestCase` class, `mock.patch.dict`
wraps each `test_*` method individually, which does restore `os.environ` after
each test, *unless* other code or sub-hooks run before/after). However,
mutating `os.environ` directly in `test_default_universe_domain`,
`test_universe_domain_from_env_var`, and
`test_universe_domain_from_credentials` is brittle and inconsistent with
`pipeline_options_test.py`. Consider using `@mock.patch.dict(os.environ,
{"GOOGLE_CLOUD_UNIVERSE_DOMAIN": _TPC_UNIVERSE_DOMAIN})` (or `with
mock.patch.dict(...)`) for clarity and guaranteed isolation.
--
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]
