fix: address code review feedback for GAPIC centralization

Author: hebaalazzeh

packages/google-api-core/google/api_core/gapic_v1/__init__.py

@@ -12,22 +12,20 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from google.api_core.gapic_v1 import client_info
-from google.api_core.gapic_v1 import routing_header
+from google.api_core.gapic_v1 import (
+    client_info,
+    config,
+    config_async,
+    method,
+    method_async,
+    routing_header,
+)
 
 __all__ = [
     "client_info",
+    "config",
+    "config_async",
+    "method",
+    "method_async",
     "routing_header",
 ]
-
-try:
-    import grpc  # noqa: F401
-
-    from google.api_core.gapic_v1 import config  # noqa: F401
-    from google.api_core.gapic_v1 import config_async  # noqa: F401
-    from google.api_core.gapic_v1 import method  # noqa: F401
-    from google.api_core.gapic_v1 import method_async  # noqa: F401
-
-    __all__.extend(["config", "config_async", "method", "method_async"])
-except ImportError:  # pragma: NO COVER
-    pass

packages/google-api-core/google/api_core/gapic_v1/_client_cert.py

@@ -17,12 +17,12 @@
 """Helpers for client certificate handling and mTLS authentication."""
 
 import os
-from typing import Any, Optional
+from typing import Callable, Optional, Tuple
 
 from google.auth.transport import mtls  # type: ignore
 
 
-def use_client_cert_effective() -> bool:
+def _use_client_cert_effective() -> bool:
     """Returns whether client certificate should be used for mTLS if the
     google-auth version supports should_use_client_cert automatic mTLS
     enablement.
@@ -53,9 +53,10 @@ def use_client_cert_effective() -> bool:
         return use_client_cert_str == "true"
 
 
-def get_client_cert_source(
-    provided_cert_source: Optional[Any], use_cert_flag: bool
-) -> Optional[Any]:
+def _get_client_cert_source(
+    provided_cert_source: Optional[Callable[[], Tuple[bytes, bytes]]],
+    use_cert_flag: bool,
+) -> Optional[Callable[[], Tuple[bytes, bytes]]]:
     """Return the client cert source to be used by the client.
 
     Args:

packages/google-api-core/google/api_core/gapic_v1/_config_helpers.py

@@ -19,11 +19,12 @@
 import os
 from typing import Optional, Tuple
 
-from google.api_core.gapic_v1._client_cert import use_client_cert_effective
 from google.auth.exceptions import MutualTLSChannelError  # type: ignore
 
+from google.api_core.gapic_v1._client_cert import _use_client_cert_effective
 
-def read_environment_variables() -> Tuple[bool, str, Optional[str]]:
+
+def _read_environment_variables() -> Tuple[bool, str, Optional[str]]:
     """Returns the environment variables used by the client.
 
     Returns:
@@ -38,7 +39,7 @@ def read_environment_variables() -> Tuple[bool, str, Optional[str]]:
             GOOGLE_API_USE_MTLS_ENDPOINT is not any of
             ["auto", "never", "always"].
     """
-    use_client_cert = use_client_cert_effective()
+    use_client_cert = _use_client_cert_effective()
     use_mtls_endpoint = os.getenv("GOOGLE_API_USE_MTLS_ENDPOINT", "auto").lower()
     universe_domain_env = os.getenv("GOOGLE_CLOUD_UNIVERSE_DOMAIN")
     if use_mtls_endpoint not in ("auto", "never", "always"):

packages/google-api-core/google/api_core/gapic_v1/_method_helpers.py

@@ -20,7 +20,7 @@
 from typing import Any
 
 
-def setup_request_id(request: Any, field_name: str, is_proto3_optional: bool) -> None:
+def _setup_request_id(request: Any, field_name: str, is_proto3_optional: bool) -> None:
     """Populate a UUID4 field in the request if it is not already set.
 
     Args:

packages/google-api-core/google/api_core/gapic_v1/_routing.py

@@ -27,7 +27,7 @@
 )
 
 
-def get_default_mtls_endpoint(api_endpoint: Optional[str]) -> Optional[str]:
+def _get_default_mtls_endpoint(api_endpoint: Optional[str]) -> Optional[str]:
     """Converts api endpoint to mTLS endpoint.
 
     Convert "*.sandbox.googleapis.com" and "*.googleapis.com" to
@@ -57,7 +57,7 @@ def get_default_mtls_endpoint(api_endpoint: Optional[str]) -> Optional[str]:
     return api_endpoint.replace(".googleapis.com", ".mtls.googleapis.com")
 
 
-def get_api_endpoint(
+def _get_api_endpoint(
     api_override: Optional[str],
     client_cert_source: Optional[Any],
     universe_domain: str,
@@ -74,8 +74,7 @@ def get_api_endpoint(
     ):
         if universe_domain.lower() != default_universe.lower():
             raise MutualTLSChannelError(
-                f"mTLS is not supported in any universe other than "
-                f"{default_universe}."
+                f"mTLS is not supported in any universe other than {default_universe}."
             )
         return default_mtls_endpoint
     else:
@@ -86,7 +85,7 @@ def get_api_endpoint(
         )
 
 
-def get_universe_domain(
+def _get_universe_domain(
     client_universe_domain: Optional[str],
     universe_domain_env: Optional[str],
     default_universe: str,

packages/google-api-core/tests/unit/gapic/test_client_cert.py

@@ -18,21 +18,20 @@
 
 import pytest
 
-
 from google.api_core.gapic_v1._client_cert import (
-    get_client_cert_source,
-    use_client_cert_effective,
+    _get_client_cert_source,
+    _use_client_cert_effective,
 )
 
 
 @mock.patch("google.auth.transport.mtls.should_use_client_cert", create=True)
 def test_use_client_cert_effective_with_google_auth(mock_method):
     # Test when google-auth supports the method
     mock_method.return_value = True
-    assert use_client_cert_effective() is True
+    assert _use_client_cert_effective() is True
 
     mock_method.return_value = False
-    assert use_client_cert_effective() is False
+    assert _use_client_cert_effective() is False
 
 
 @mock.patch.dict(os.environ, {}, clear=True)
@@ -42,23 +41,23 @@ def test_use_client_cert_effective_fallback():
         "google.api_core.gapic_v1._client_cert.hasattr", return_value=False
     ):
         # Default is false
-        assert use_client_cert_effective() is False
+        assert _use_client_cert_effective() is False
 
         env_true = {"GOOGLE_API_USE_CLIENT_CERTIFICATE": "true"}
         with mock.patch.dict(os.environ, env_true):
-            assert use_client_cert_effective() is True
+            assert _use_client_cert_effective() is True
 
         with mock.patch.dict(
             os.environ, {"GOOGLE_API_USE_CLIENT_CERTIFICATE": "false"}
         ):
-            assert use_client_cert_effective() is False
+            assert _use_client_cert_effective() is False
 
         with mock.patch.dict(
             os.environ, {"GOOGLE_API_USE_CLIENT_CERTIFICATE": "invalid"}
         ):
             match_str = "must be either `true` or `false`"
             with pytest.raises(ValueError, match=match_str):
-                use_client_cert_effective()
+                _use_client_cert_effective()
 
 
 @mock.patch(
@@ -72,10 +71,10 @@ def test_get_client_cert_source(mock_default, mock_has_default):
     mock_has_default.return_value = True
 
     # When use_cert_flag is False, return None
-    assert get_client_cert_source(b"provided", False) is None
+    assert _get_client_cert_source(b"provided", False) is None
 
     # When provided_cert_source is given, return provided
-    assert get_client_cert_source(b"provided", True) == b"provided"  # noqa: E501
+    assert _get_client_cert_source(b"provided", True) == b"provided"  # noqa: E501
 
     # When no provided cert but default is available
-    assert get_client_cert_source(None, True) == b"default_cert"
+    assert _get_client_cert_source(None, True) == b"default_cert"

packages/google-api-core/tests/unit/gapic/test_config_helpers.py

@@ -18,22 +18,21 @@
 
 import pytest
 
-
 from google.auth.exceptions import MutualTLSChannelError
 
-from google.api_core.gapic_v1._config_helpers import read_environment_variables
+from google.api_core.gapic_v1._config_helpers import _read_environment_variables
 
 
 @mock.patch(
-    "google.api_core.gapic_v1._config_helpers.use_client_cert_effective"
+    "google.api_core.gapic_v1._config_helpers._use_client_cert_effective"
 )  # noqa: E501
 @mock.patch.dict(os.environ, clear=True)
 def test_read_environment_variables(mock_effective):
     mock_effective.return_value = True
     os.environ["GOOGLE_API_USE_MTLS_ENDPOINT"] = "always"
     os.environ["GOOGLE_CLOUD_UNIVERSE_DOMAIN"] = "custom.com"
 
-    cert, mtls, domain = read_environment_variables()
+    cert, mtls, domain = _read_environment_variables()
     assert cert is True
     assert mtls == "always"
     assert domain == "custom.com"
@@ -45,4 +44,4 @@ def test_read_environment_variables_invalid_mtls():
     with pytest.raises(
         MutualTLSChannelError, match="must be `never`, `auto` or `always`"
     ):
-        read_environment_variables()
+        _read_environment_variables()

packages/google-api-core/tests/unit/gapic/test_method_helpers.py

@@ -14,22 +14,22 @@
 # limitations under the License.
 
 
-from google.api_core.gapic_v1._method_helpers import setup_request_id
+from google.api_core.gapic_v1._method_helpers import _setup_request_id
 
 
 def test_setup_request_id():
     import uuid
 
     # test dict request
     req = {}
-    setup_request_id(req, "request_id", True)
+    _setup_request_id(req, "request_id", True)
     assert "request_id" in req
     uuid_str = req["request_id"]
     uuid.UUID(uuid_str)  # verify it is a valid UUID
 
     # test dict request when already set
     req = {"request_id": "existing"}
-    setup_request_id(req, "request_id", True)
+    _setup_request_id(req, "request_id", True)
     assert req["request_id"] == "existing"
 
     class DummyRequest:
@@ -43,12 +43,22 @@ def HasField(self, field_name):
 
     # test object request proto3 optional true
     req_obj = DummyRequest()
-    setup_request_id(req_obj, "request_id", True)
+    _setup_request_id(req_obj, "request_id", True)
     assert req_obj.request_id != ""
     uuid.UUID(req_obj.request_id)
 
     # test object request proto3 optional false
     req_obj2 = DummyRequest()
-    setup_request_id(req_obj2, "request_id", False)
+    _setup_request_id(req_obj2, "request_id", False)
     assert req_obj2.request_id != ""
     uuid.UUID(req_obj2.request_id)
+
+    class CustomRequestWrapper:
+        def __init__(self):
+            self.request_id = ""
+
+    # test custom non-iterable object wrapper
+    req_obj3 = CustomRequestWrapper()
+    _setup_request_id(req_obj3, "request_id", True)
+    assert req_obj3.request_id != ""
+    uuid.UUID(req_obj3.request_id)