refactor(gapic-generator): deduplicate sync/async prep_wrapped_messages and transport method wrapping
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 35/100
- issue の種類
- リファクタリング
- 明瞭さ
- 説明が足りない
- 活発さ
- 活発
- 技術スタック
- python
- 領域
- build-system, tooling
調査の方向性
Start by comparing the synchronous implementation in packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2 with prep_wrapped_messages_async_method in _shared_macros.j2, then review the related PR #18433. Decide among the proposed consolidation approaches and verify that generated sync, AsyncIO gRPC, REST, and AsyncIO REST transports retain their wrapping and retry behavior without the duplicated templates.
索引モデルが issue の本文から書いたものです。
説明
Problem Summary
In packages/gapic-generator/gapic/templates/, there is code duplication and structural asymmetry between synchronous and asynchronous RPC preparation and method wrapping across the generated transport classes:
-
_prep_wrapped_messagesDuplication:packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2(lines 239–280) defines_prep_wrapped_messages(self, client_info)for synchronous transports.packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2(lines 322–371) definesprep_wrapped_messages_async_method(api, service)for asynchronous transports.- These two blocks are ~50 lines of Jinja template code that are nearly identical. Both iterate over
service.methods.values()andapi.mixin_api_methods.keys(), mapping every RPC to its retry predicate, default timeout, client info, fully-qualified method name, and streaming flags. The only differences are:self._wrap_methodvsself._wrap_async_methodretries.Retry(...)vsretries.AsyncRetry(...)
-
_wrap_methodvs_wrap_async_methodDuplication:- Both methods are defined side-by-side in
base.py.j2(lines 174–237). - Both methods implement identical compatibility checks and argument filtering across three generations of
google-api-core(client_options,kind,method_name,is_streaming), delegating respectively togapic_v1.method.wrap_methodandgapic_v1.method_async.wrap_method. - Consider moving these to
_compat.pyto reduce the footprint in generated code.
- Both methods are defined side-by-side in
-
Architectural Asymmetry (Base Class vs Shared Macros):
_wrap_async_methodis defined directly on the synchronous base class{{ service.name }}Transportinbase.py.j2, even though synchronous transports never call it.- Conversely, asynchronous
_prep_wrapped_messagesis not inbase.py.j2—it lives as an external Jinja macro in_shared_macros.j2that is injected intogrpc_asyncio.py.j2andrest_asyncio.py.j2to override the base class implementation. - This mixing makes the transport hierarchy harder to reason about and causes maintenance drift when retry parameters, metadata, or tracing attributes are updated.
Deduplication Options to Consider
Option A: Polymorphic Dispatch in Base Transport (Recommended)
Consolidate _prep_wrapped_messages into a single, unified implementation in base.py.j2 by making transport classes declare their retry class and RPC wrapper callable:
# In base.py.j2 ({{ service.name }}Transport):
_is_async: bool = False
_retry_type = retries.Retry
def _wrap_rpc(self, func, *args, **kwargs):
# Unified wrap helper parameterized by sync vs async
wrap_func = gapic_v1.method_async.wrap_method if self._is_async else gapic_v1.method.wrap_method
supports_tracing = _ASYNC_WRAP_METHOD_SUPPORTS_TRACING if self._is_async else _WRAP_METHOD_SUPPORTS_TRACING
if supports_tracing:
kwargs["client_options"] = self._client_options
if self.kind:
kwargs["kind"] = self.kind
return wrap_func(func, *args, **kwargs)
...
- Pros:
_prep_wrapped_messagesonly exists once inbase.py.j2. All transports (gRPC, AsyncIO gRPC, REST, AsyncIO REST) inherit it naturally.- Deletes
prep_wrapped_messages_async_methodfrom_shared_macros.j2entirely (~50 lines removed). - Eliminates duplicate template loops for mixin and service methods.
- Cons: Requires setting transport class-level attributes (
_is_async,_retry_type) on async transports.
Option B: Parameterized Jinja Macro
Unify synchronous and asynchronous message preparation into a single reusable Jinja macro in _shared_macros.j2:
{% macro prep_wrapped_messages(api, service, is_async=False) %}
def _prep_wrapped_messages(self, client_info):
self._wrapped_methods = {
{% for method in service.methods.values() %}
self.{{ method.transport_safe_name|snake_case }}: self.{{ "_wrap_async_method" if is_async else "_wrap_method" }}(
...
{% if method.retry %}
default_retry=retries.{{ "AsyncRetry" if is_async else "Retry" }}(...),
{% endif %}
...
),
{% endfor %}
}
{% endmacro %}
- Pros:
- Keeps generation purely in templates without modifying Python transport class contracts or inheritance semantics.
- Single source of truth for method iteration and configuration.
- Cons: Still renders two copies of the method across generated files; Jinja syntax becomes slightly more branchy.
Option C: Dedicated AsyncTransport Base Class
Introduce an explicit _Base{{ service.name }}AsyncTransport class (similar to _Base{{ service.name }}RestTransport):
-
{{ service.name }}Transport: Sync base class containing sync_prep_wrapped_messagesand_wrap_method. -
_Base{{ service.name }}AsyncTransport: Async base class containing async_prep_wrapped_messagesand_wrap_async_method. -
Pros:
- Clean separation of concerns; async wrappers are no longer baked into sync base classes.
- Idiomatic object-oriented hierarchy.
-
Cons:
- Larger structural change across generated codebases and public type contracts.
- Requires updating multiple transport inheritance graphs.
Related Discussion
- PR #18433:
feat(observability): [WIP] implement universal 4-path OpenTelemetry tracing - Follow-up from reviewer feedback regarding code duplication between sync and async transports.
- 主要言語
- Python
- スター
- 5.4k
- フォーク
- 1.8k
- 平均マージ
- 2日 7時間
- マージ済み PR(30日)
- 173
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートあり
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
googleapis/google-cloud-python のほかの issue
-
firestore: field path with a quoted name ending in a backslash is misparsed対応中かも @sebxt が 25 日前に担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
googleapis/google-cloud-python#18771 ·
メンテナーはふだん 1 日以内に返信
-
api: spanner type: feature request
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
googleapis/google-cloud-python#18584 ·
メンテナーはふだん 1 日以内に返信
-
auth: `impersonated_credentials` loses `subject` on copy, so `with_scopes()` and `with_quota_project()` turn off domain wide delegation対応中かも @Om-singhaI が 55 日前に担当しました。 オープンauth priority: p2 type: bug
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
googleapis/google-cloud-python#18428 ·
メンテナーはふだん 1 日以内に返信
-
A client called `run_in_transaction` be stuck if Spanner is down対応中かも @founderinaction が 7 日前に担当しました。 オープンpriority: p2 type: bug
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
googleapis/google-cloud-python#18375 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
ci: enable --strict flag in presubmit twine_check再び着手できるかも @ohmayr が 44 日前に担当しましたが、オープン中のプルリクエストはありません。 オープン
難易度 1/5 1時間未満 初心者へのやさしさ 76/100
googleapis/google-cloud-python#18339 · コメント 1 件 · 担当者 1 名 ·
メンテナーはふだん 1 日以内に返信
googleapis/google-cloud-python の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
cpp-best-practices/project_validator#267 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
Project-N-E-K-O/N.E.K.O#3725 ·
メンテナーはふだん 1 日以内に返信
-
[Bug] Sending a Telegram message chain twice duplicates mentions対応中かも @123XieC が今日担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
AstrBotDevs/AstrBot#11125 ·
メンテナーはふだん 1 日以内に返信
-
backend bug priority: high
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
nooriegarg/ContribIQ#73 ·
メンテナーはふだん 1 日以内に返信
-
bug p3 security
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
メンテナーはふだん 1 日以内に返信