Skip to content

chore: complete migration of google-cloud-biglake-hive#16684

Open
jskeet wants to merge 1 commit intogoogleapis:mainfrom
jskeet:migrate-biglake-hive
Open

chore: complete migration of google-cloud-biglake-hive#16684
jskeet wants to merge 1 commit intogoogleapis:mainfrom
jskeet:migrate-biglake-hive

Conversation

@jskeet
Copy link
Copy Markdown
Contributor

@jskeet jskeet commented Apr 16, 2026

The code changes here are due to the package not having been regenerated with the latest version of legacylibrarian.

Fixes googleapis/librarian#5336

The code changes here are due to the package not having been regenerated
with the latest version of legacylibrarian.

Fixes googleapis/librarian#5336
@jskeet jskeet requested a review from parthea April 16, 2026 08:38
@jskeet jskeet requested a review from a team as a code owner April 16, 2026 08:38
Copy link
Copy Markdown
Contributor

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

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 library to support Python 3.9+ and drops support for Python 3.7, 3.8, and Protobuf 3.x. Changes include updated metadata, documentation, and Nox configurations, as well as the addition of type hints and refined mTLS endpoint resolution. Tests were modernized using parenthesized context managers. Review feedback suggests adding a type hint to the api_endpoint parameter and using built-in collection types for type hinting as per PEP 585.


@staticmethod
def _get_default_mtls_endpoint(api_endpoint):
def _get_default_mtls_endpoint(api_endpoint) -> Optional[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 parameter api_endpoint is missing a type hint. Since this PR adds a return type hint to this method, the parameter should also be hinted for consistency and better type checking. Based on the docstring, the type is Optional[str].

Suggested change
def _get_default_mtls_endpoint(api_endpoint) -> Optional[str]:
def _get_default_mtls_endpoint(api_endpoint: Optional[str]) -> Optional[str]:
References
  1. PEP 484 – Type Hints (link)

host += ":443"
self._host = host

self._wrapped_methods: Dict[Callable, Callable] = {}
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

Since the package now requires Python 3.9+, you should use the built-in dict for type hinting instead of typing.Dict, as per PEP 585.

Suggested change
self._wrapped_methods: Dict[Callable, Callable] = {}
self._wrapped_methods: dict[Callable, Callable] = {}
References
  1. PEP 585 – Type Hinting Generics In Standard Collections (link)

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.

python: fix and enable generation for google-cloud-biglake-hive

1 participant