-
Notifications
You must be signed in to change notification settings - Fork 89
chore: Force grpcio-tools dev dependency to <1.66.0 to allow grpcio compatibility with <1.66.0 #270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a compatibility issue with gRPC dependencies by constraining the grpcio-tools
dev dependency to versions below 1.66.0, allowing the library to work with projects that require grpcio
< 1.66.0. The change resolves a RuntimeError that was introduced in version 24.0.2 when protobuf sources were regenerated with newer grpcio-tools
.
- Constrained
grpcio-tools
dev dependency from>=1.68.0
to>=1.65.0,<1.66.0
- Regenerated protobuf source files to use older gRPC tooling that emits warnings instead of RuntimeErrors
- Updated changelog to document the compatibility fix
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
pyproject.toml | Updated grpcio-tools version constraint with explanatory comment |
pydgraph/proto/api_pb2_grpc.py | Regenerated gRPC code that uses warnings instead of RuntimeError for version mismatches |
pydgraph/proto/api_pb2.py | Regenerated protobuf code with older tooling version |
CHANGELOG.md | Added changelog entry documenting the compatibility fix |
…ompatibility with <1.66.0
e9fa9fa
to
ab54fb2
Compare
Looks like the Python 3.13 CI test breaks because grpc only added support for it in 1.66.2... |
Right, we discussing internally dropping 3.13 support. Don't think many people are using in production as 3.12 is the official "LTS" |
[PATCH 1/3] Remove python 3.13 from testing; update docs [PATCH 2/3] Add a function to display current Dgraph version active in tests [PATCH 3/3] Bump pydgraph version
2ed8f9d
to
f6e1da4
Compare
Note: I successfully tested this pydgraph branch against Dgraph versions: v25.0.0-preview6, v24.1.2 and v23.1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please hold on this until we've had a chance to discuss internally. Thanks.
I don't think you necessarily need to drop Python 3.13 support - I've pushed a commit that readds support for Python 3.13, but skips the protogen diff step in the CI tests for 3.13. |
Ah, of course. Thanks for pointing that out... the diff checks will happen in the other build steps. |
Hi folks. This PR doesn't sit well with me, for a few reasons:
I'm closing this PR. We should take a closer look at solving the underlying issue, such that we can properly support 3.12 and earlier, as well as 3.13 (and newer, as time progresses). If that means we have to branch the code somewhere, or the workflow, that's fine - but we can't simply sidestep it. Let's work on that in a new PR. Thanks. |
This has been reimplemented in #272 |
Description
This PR bumps down the dev dependency
grpcio-tools
to <1.66.0 to allowgrpcio
compatibility with <1.66.0.This compatibility seems to have been broken in pydgraph 24.0.2 by #255 (where regenerating the protobuf sources with
grpcio-tools
>= 1.66.0 changed the warning in api_pb2_grpc.py to a RuntimeError).Example error:
Addresses #222, related to #233
Changes
grpcio-tools
dev dependency fromgrpcio-tools>=1.68.0
togrpcio-tools>=1.65.0,<1.66.0
python scripts/protogen.py
)Rationale (adapted from #233)
grpcio
< 1.66.0 due to this open issue with tests (since September 2024): [Python] Significant test flakiness after upgrading to grpcio 1.66.0 grpc/grpc#37710grpcio-tools
and the newer version, meaning this forced version bump is not currently requiredChecklist
CHANGELOG.md
file describing and linking tothis PR
docs repo staged and linked here