Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request ensures that batch request URIs correctly respect custom API endpoints, mTLS endpoints, and universe domains by using the base URL in googleapiclient/discovery.py. It also adds comprehensive unit tests to verify these scenarios. The feedback suggests improving code readability by using f-strings instead of % formatting for string interpolation.
| # base URL so the batch follows the universe domain, api_endpoint | ||
| # and mTLS endpoint the same way every other request does. | ||
| batch_uri = _fix_up_media_path_base_url( | ||
| "%s%s" % (rootDesc["rootUrl"], rootDesc.get("batchPath", "batch")), |
There was a problem hiding this comment.
In accordance with the general rules, f-strings are preferred over % formatting for string interpolation in Python.
Suggested change
| "%s%s" % (rootDesc["rootUrl"], rootDesc.get("batchPath", "batch")), | |
| f"{rootDesc['rootUrl']}{rootDesc.get('batchPath', 'batch')}", |
References
- Prefer using f-strings for string interpolation in Python.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
new_batch_http_request()builds its endpoint asrootUrl+batchPathstraight from the discovery document, while every ordinary method request goes throughself._baseUrl, which already carries the universe domain substitution, theclient_options.api_endpointoverride and the mTLS endpoint choice. A client configured for a non-default universe, a restricted or privateapi_endpoint, orGOOGLE_API_USE_MTLS_ENDPOINT=alwaystherefore POSTs its batch, with each sub-request's bearer token inside the multipart body, to the publicwww.googleapis.comhost. The universe check in_validate_credentialsdoes not catch this because it compares the configured universes rather than the host the batch is actually sent to. The batch URI now takes its host from_baseUrlthrough the same_fix_up_media_path_base_urlcorrection that media upload URLs already get, and the three new tests fail onmainand pass with the change.