Skip to content

feat(api-core): centralize rest transcoding helpers#17765

Open
hebaalazzeh wants to merge 1 commit into
mainfrom
feat/gapic-centralization-api-core-transcoding
Open

feat(api-core): centralize rest transcoding helpers#17765
hebaalazzeh wants to merge 1 commit into
mainfrom
feat/gapic-centralization-api-core-transcoding

Conversation

@hebaalazzeh

@hebaalazzeh hebaalazzeh commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

introduces a generic REST Transcoding helper under google.api_core.gapic_v1.rest_transcoding.

Scope of Changes:

  • rest_helpers.py: A centralized, parameters-driven generic utility transcode() that replaces the redundant code generated inside the client REST transport constructors (query parameter JSON parsing, required field default checking, body parsing, and numeric enum serialization).
  • __init__.py: Registered lazy-loading for the module.
  • conftest.py: Added session-scoped mock fixtures for mTLS variables to prevent local workstation client certificate leakage during python unit tests.
  • test_rest_transcoding.py: A comprehensive test suite validating transcoding behavior for query parameters, nested fields, required field default injection, and numeric enums.

This is the runtime package companion to the generator PR.

@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 introduces a new rest_transcoding module under google/api_core/gapic_v1 to handle transcoding of requests into HTTP methods, URIs, bodies, and query parameters. It also adds comprehensive unit tests for this new functionality, configures a session-scoped pytest fixture to isolate tests from workstation mTLS environments, and updates Python version configurations. The feedback suggests optimizing the conversion of protobuf messages to Python dictionaries in rest_transcoding.py by using json_format.MessageToDict instead of serializing to JSON and deserializing it back.

Comment thread packages/google-api-core/google/api_core/gapic_v1/rest_helpers.py Outdated
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-transcoding branch from 3af2bfe to 0b05ab3 Compare July 17, 2026 23:37
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-transcoding branch 2 times, most recently from 2e4481a to c736fcd Compare July 18, 2026 00:27
@hebaalazzeh

Copy link
Copy Markdown
Contributor Author

/gemini review

@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 introduces a new helper module rest_helpers in google.api_core.gapic_v1 containing a transcode function, which converts protobuf or proto-plus request messages into HTTP methods, URIs, JSON bodies, and query parameters. It also includes comprehensive unit tests for this function and a new session-scoped test fixture to isolate unit tests from workstation mTLS environments. There are no review comments, and I have no additional feedback to provide.

@hebaalazzeh
hebaalazzeh marked this pull request as ready for review July 18, 2026 00:47
@hebaalazzeh
hebaalazzeh requested a review from a team as a code owner July 18, 2026 00:47
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-transcoding branch 2 times, most recently from e009a6b to 81ad231 Compare July 18, 2026 06:01
@daniel-sanche

daniel-sanche commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
  1. Can you point to the code that this is replacing in a generated client? I'm having toruble finding it
  2. It could be confusing that this method has the same name as path_template.transcode, which this is built on top of

Edit: I guess this is replacing a number of methods in rest_base? Can you show a diff of what the changes will look like in the generated code?

@hebaalazzeh

hebaalazzeh commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author
  1. Can you point to the code that this is replacing in a generated client? I'm having toruble finding it
  2. It could be confusing that this method has the same name as path_template.transcode, which this is built on top of

Edit: I guess this is replacing a number of methods in rest_base? Can you show a diff of what the changes will look like in the generated code?

Regarding the code this is replacing: This is replacing the three private helper methods generated for each REST RPC: _get_transcoded_request, _get_request_body_json, and _get_query_params_json (defined in rest_base.py.j2:L128-167 and originally called within the template at _shared_macros.j2:L209-216). By consolidating them, we avoid generating duplicate request serialization, JSON body conversion, and default required field query parameter injection logic for every REST endpoint.

Regarding the method name: Great point. I've renamed transcode in rest_helpers.py to transcode_request to make it clear that it transcodes a full request and to avoid any confusion with path_template.transcode.

for a diff example:

# rest_base.py
    class _BaseEcho:
        @staticmethod
        def _get_http_options():
            return [{'method': 'post', 'uri': '/v1beta1/echo:echo', 'body': '*'}]

-       @staticmethod
-       def _get_transcoded_request(http_options, request):
-           pb_request = request
-           return path_template.transcode(http_options, pb_request)
-
-       @staticmethod
-       def _get_request_body_json(transcoded_request):
-           return json_format.MessageToJson(transcoded_request['body'], use_integers_for_enums=False)
-
-       @staticmethod
-       def _get_query_params_json(transcoded_request):
-           return json.loads(json_format.MessageToJson(transcoded_request['query_params'], use_integers_for_enums=False))
# rest.py
-           transcoded_request = _BaseEchoRestTransport._BaseEcho._get_transcoded_request(http_options, request)
-           body = _BaseEchoRestTransport._BaseEcho._get_request_body_json(transcoded_request)
-           query_params = _BaseEchoRestTransport._BaseEcho._get_query_params_json(transcoded_request)
+           transcoded_request, body, query_params = rest_helpers.transcode_request(
+               http_options,
+               request,
+               required_fields_default_values=getattr(
+                   _BaseEchoRestTransport._BaseEcho,
+                   "__REQUIRED_FIELDS_DEFAULT_VALUES",
+                   None,
+               ),
+               rest_numeric_enums=False,
+           )

@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-transcoding branch from 81ad231 to 04a1519 Compare July 21, 2026 14:33
@snippet-bot

snippet-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-transcoding branch 2 times, most recently from 1706e20 to 3ff39e7 Compare July 21, 2026 14:40
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-transcoding branch 2 times, most recently from f399974 to 509c733 Compare July 21, 2026 16:43
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-transcoding branch from 509c733 to 2c3a593 Compare July 21, 2026 16:55
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.

2 participants