Skip to content

WIP - #17999

Draft
hebaalazzeh wants to merge 1 commit into
mainfrom
fallback-api-core
Draft

WIP#17999
hebaalazzeh wants to merge 1 commit into
mainfrom
fallback-api-core

Conversation

@hebaalazzeh

Copy link
Copy Markdown
Contributor

revisit once new version of api core released

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the templates to conditionally import several helper functions (get_default_mtls_endpoint, get_api_endpoint, get_universe_domain, and transcode_request) from google.api_core if available, falling back to local implementations on ImportError. It also updates the client template to explicitly pass default_universe="googleapis.com" to get_universe_domain. The feedback correctly points out that the fallback implementation of get_universe_domain in _compat.py.j2 is missing the default value for default_universe, which breaks compatibility with the original signature and the google.api_core version.

Comment on lines +153 to +156
def get_universe_domain(
*potential_universes: Optional[str],
default_universe: str,
) -> str:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The fallback implementation of get_universe_domain is missing the default value for default_universe (default_universe: str = DEFAULT_UNIVERSE). This breaks compatibility with the original signature and with google.api_core.universe.get_universe_domain, which makes default_universe optional. Please restore the default value to ensure it remains a drop-in replacement.

    def get_universe_domain(
        *potential_universes: Optional[str],
        default_universe: str = DEFAULT_UNIVERSE,
    ) -> str:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant