Skip to content

chore(generator): move setup_request_id into compat layer#17739

Open
hebaalazzeh wants to merge 100 commits into
mainfrom
feat/gapic-generator-centralization-request-id
Open

chore(generator): move setup_request_id into compat layer#17739
hebaalazzeh wants to merge 100 commits into
mainfrom
feat/gapic-generator-centralization-request-id

Conversation

@hebaalazzeh

@hebaalazzeh hebaalazzeh commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Moves setup_request_id defined in the client template into _compat.py.j2 and the related test cases from test_%service.py.j2 to test_compat.py.j2

Associated / Dependent API Core PR: #17738
Towards: #17883

@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 refactors the request ID setup logic in the generated client templates by delegating it to google.api_core.gapic_v1.method_helpers.setup_request_id instead of maintaining an inline implementation. As a result, the redundant unit tests for _setup_request_id have been removed from both the templates and the golden integration tests. There are no review comments, so I have no feedback to provide.

@hebaalazzeh
hebaalazzeh changed the base branch from main to feat/gapic-centralization-api-core-request-id July 16, 2026 15:20
@hebaalazzeh hebaalazzeh self-assigned this Jul 16, 2026
@hebaalazzeh hebaalazzeh reopened this Jul 16, 2026
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch from f037044 to f25d768 Compare July 16, 2026 16:10
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch from a0bff77 to 613b392 Compare July 16, 2026 16:18
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch from f25d768 to 71b2f4e Compare July 16, 2026 16:20
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch 3 times, most recently from e02ee4b to db040bb Compare July 16, 2026 16:28
@daniel-sanche daniel-sanche added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 16, 2026
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch from 71b2f4e to a8cc8d8 Compare July 16, 2026 18:11
@daniel-sanche

Copy link
Copy Markdown
Contributor

This implementation makes sense if we plan to bump up the api_core requirement Before merging, we need to:

  1. get the changes in feat(api_core): add request-id auto-population logic to gapic_v1 public helpers #17738 released
  2. update the setup.py to reference the new api_core version floor
  3. regenerate goldens

There's also a chance we will want to keep a fallback implementation to keep supporting old api_core versions

@hebaalazzeh hebaalazzeh changed the title feat: delegate request-id setup to public method_helpers chore(generator): delegate request-id setup to public request Jul 16, 2026
…lic helpers

Introduces method_helpers module containing setup_request_id helper. Exposes the module as public in gapic_v1.
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch from 90602cc to 76ac456 Compare July 16, 2026 23:21
…lic helpers

Introduces method_helpers module containing setup_request_id helper. Exposes the module as public in gapic_v1.
Updates generator templates and goldens to import public method_helpers from google-api-core gapic_v1 and call setup_request_id helper. Removes duplicate setup_request_id test logic from generated client unit tests.
@hebaalazzeh
hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch from 8e31d83 to 20adea6 Compare July 16, 2026 23:42
Comment thread packages/gapic-generator/noxfile.py Outdated
@hebaalazzeh

Copy link
Copy Markdown
Contributor Author

To simplify things for myself and the review process I am going to break things down:

@hebaalazzeh hebaalazzeh changed the title chore(generator): delegate request-id setup to public request chore(generator): move setup_request_id into compat layer Jul 24, 2026

@ohmayr ohmayr 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.

@hebaalazzeh I've added some suggestions for generating these methods conditionally. This'll avoid generating these helper methods in libraries which don't need them.

You can regenerate the goldens and you shouldn't see a diff in any of them except storagebatchoperations which has auto populated fields. PTAL.

hebaalazzeh and others added 6 commits July 24, 2026 21:32
…sion/%sub/_compat.py.j2

Co-authored-by: ohmayr <omairnaveed@ymail.com>
…e_%version/%sub/test_compat.py.j2

Co-authored-by: ohmayr <omairnaveed@ymail.com>
…e_%version/%sub/test_compat.py.j2

Co-authored-by: ohmayr <omairnaveed@ymail.com>
…sion/%sub/_compat.py.j2

Co-authored-by: ohmayr <omairnaveed@ymail.com>
…e_%version/%sub/test_compat.py.j2

Co-authored-by: ohmayr <omairnaveed@ymail.com>
…sion/%sub/_compat.py.j2

Co-authored-by: ohmayr <omairnaveed@ymail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants