Skip to content

fix(python): soften grpc generated version check in proto stubs - #81

Closed
Sunny (sunnylovestiramisu) wants to merge 1 commit into
agent-substrate:mainfrom
sunnylovestiramisu:grpcio-tools
Closed

Sunny (sunnylovestiramisu) wants to merge 1 commit into
agent-substrate:mainfrom
sunnylovestiramisu:grpcio-tools

Conversation

@sunnylovestiramisu

Copy link
Copy Markdown
Collaborator

Summary

Softens the version compatibility check in generated gRPC Python stubs (env_pb2_grpc.py and guest_pb2_grpc.py) from raise RuntimeError(...) to warnings.warn(...).

This prevents catastrophic, unrecoverable crashes at import ate_env in Python environments with slightly mismatched grpcio runtime versions.


Background & Problem

In recent versions of grpcio-tools, the protobuf compiler injects a version check header at the top of generated *_pb2_grpc.py files:

GRPC_GENERATED_VERSION = '1.81.1'
GRPC_VERSION = grpc.__version__
_version_not_supported = False

try:
    from grpc._utilities import first_version_is_lower
    _version_not_supported = first_version_is_lower(GRPC_VERSION, GRPC_GENERATED_VERSION)
except ImportError:
    _version_not_supported = True

if _version_not_supported:
    raise RuntimeError(
        f'The grpc package installed is at version {GRPC_VERSION},'
        ...
    )

Change raise RuntimeError to warnings.warn in env_pb2_grpc.py and
guest_pb2_grpc.py to prevent hard crashes when importing in environments
with slightly different grpcio versions.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes. The PR edits generated files to hide a dependency conflict instead of fixing the conflict.

Blocking

  1. clients/python/src/ate_env/_gen/ateenv/v1alpha/env_pb2_grpc.py:31 and guest_pb2_grpc.py:31 — these files are generated. The banner on line 15 says DO NOT EDIT, and clients/python/scripts/gen-protos.sh overwrites them. The next proto change reverts this PR silently. A fix must live in the generator inputs or in pyproject.toml.

  2. Same lines — the check guards a real incompatibility. The stubs call channel.unary_unary(..., _registered_method=True) in eight places in env_pb2_grpc.py. A runtime older than the generator rejects that keyword. With warnings.warn, import ate_env succeeds and the first RPC fails with a TypeError far from the cause. The RuntimeError names both versions and both remedies. Python shows a UserWarning once, and many test runners filter it.

  3. clients/python/pyproject.toml:27 — the package already requires grpcio>=1.83, above the generated 1.81.1, and integrations/nemo-gym/pyproject.toml:28 requires the same. An environment that reaches this check installed grpcio below the package's own bound, and pip reports that as a conflict. The PR must name that environment and its grpcio version. The fix belongs there, or in this package's bound, not in the stubs.

Before merge, if an older runtime must be supported

  1. Regenerate with a grpcio-tools that matches the oldest runtime to support, and lower grpcio>= in pyproject.toml to that version. GRPC_GENERATED_VERSION then equals the bound and the check passes by construction. Run gen-protos.sh with that tool pinned, and add the license headers back, because the script does not write them.

  2. Rebase onto main. The branch is one commit behind, and #70 regenerated env_pb2.py. This is also the same edit as cccfb08 on #75; drop it there once this PR decides the approach.

Nits

  1. The body states the problem as "slightly mismatched grpcio versions" but does not say which environment pins the old grpcio. Name it, because that decides between option 3 and option 4.

@sunnylovestiramisu

Copy link
Copy Markdown
Collaborator Author

Should respect the grpcio>=1.83 dependency everywhere and revert the PR1 change.

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