Bump protobuf 4.25.8 → 5.29.6 - #349
Open
reitblatt wants to merge 4 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (37)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDependency constraints are updated for protobuf, gRPC, Tink, and related Google Cloud packages. Protobuf and gRPC bindings are regenerated with runtime compatibility checks, and generated service wiring now uses registered-method APIs. ChangesProtobuf and gRPC upgrade
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
reitblatt
marked this pull request as ready for review
June 15, 2026 16:43
reitblatt
requested review from
a team,
EXBreder,
ayubun,
haileyok and
vinaysrao1
as code owners
June 15, 2026 16:43
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 11: Replace the placeholder PR link in the CHANGELOG.md file where the
protobuf upgrade is documented. Find the text
`[`#XXX`](https://github.com/roostorg/osprey/pull/XXX)` and replace both the link
text (`#XXX`) and the URL with the actual PR number of this change to create a
valid changelog reference.
In `@pyproject.toml`:
- Around line 26-28: Multiple dependency packages are being upgraded across
pyproject.toml (google-cloud-logging at lines 26-28, google-cloud-pubsub at
lines 26-28, protobuf/grpcio/tink/grpcio-tools at lines 36-41, and additional
packages at lines 56 and 77), but the PR lacks documented evidence of the
required CVE and license review approvals mandated by AGENTS.md. Add a checklist
or link in the PR description or as a comment documenting that you have verified
each upgraded package for license compatibility with LICENSE.md and confirmed
there are no known CVEs, covering all affected dependency upgrades across the
entire file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c9680a2c-49f2-4ca1-a5d4-6873e6b4beb7
⛔ Files ignored due to path filters (37)
osprey_rpc/src/osprey/rpc/actions/v1/action_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/action_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/action_types_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/action_types_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/application_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/application_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/captcha_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/captcha_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/channel_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/channel_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/guild_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/guild_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/invite_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/invite_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/metadata_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/metadata_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/user_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/user_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/execution_result_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/execution_result_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/take_data_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/take_data_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/verdicts_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/verdicts_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/etcd_watcherd/v1/etcd_watcherd_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/etcd_watcherd/v1/etcd_watcherd_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/bidirectional_stream/v1/service_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/bidirectional_stream/v1/service_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/sync_action/v1/service_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/sync_action/v1/service_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/tests/v1/tests_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/tests/v1/tests_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/v1/options_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/v1/options_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/request_caching/v1/service_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/request_caching/v1/service_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyuv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
CHANGELOG.mdpyproject.toml
Member
Member
yea this has been problematic in the past (both for me when deploying at bluesky and at discord). there are some documented issues here grpc/grpc#38327 |
Contributor
Author
Member
|
Sharing some Claude findings re: the bump of grpcio. these are current unverified in the osprey repo, but were verified via testing the library outside of osprey grpcio aio channel-churn memory leak (#38327): root cause, the fix, and how it was foundSummary
How it was found1. Reproduce, and separate the two codepathsThe issue's reproducer creates/closes a channel in a loop. Testing carefully:
The aio path is the real bug. This matches the strongest reports in the thread: a macOS aio-server 2. Identify the triggerThe leak only fires when the aio refcount drops to 0 — i.e. when the last channel is deallocated.
3. Bisect to the exact release, then the exact commit
4. Pin down the exact mechanism (no guessing)Reading the #38980 diff, the lone lifecycle change that stood out was the removal of a global
Disabling fork support makes the leak vanish → the leak is the fork-handler registry. (grpcio's Root cause in detailBefore #38980, class ObjectGroupForkHandler {
// ...
std::vector<std::weak_ptr<Forkable>> forkables_; // grows here
};
void ObjectGroupForkHandler::RegisterForkable(
std::shared_ptr<Forkable> forkable, ...) {
if (IsForkEnabled()) {
CHECK(!is_forking_);
forkables_.emplace_back(forkable); // append on every creation
// (pthread_atfork registered once)
}
}
// The ONLY place expired entries are removed is inside the fork hooks:
void ObjectGroupForkHandler::Prefork() {
if (IsForkEnabled()) {
for (auto it = forkables_.begin(); it != forkables_.end();) {
auto shared = it->lock();
if (shared) { shared->PrepareFork(); ++it; }
else { it = forkables_.erase(it); } // pruning only on fork
}
}
}These handlers are static per translation unit (e.g.
This explains every observation: aio-only (sync pins Core), hidden by concurrency (refcount stays
What in PR #38980 actually fixes itThe fix is the removal of the Concretely, commit
With no global registry, repeatedly creating and destroying EventEngines no longer accumulates
Recommendation
Reproduction / verification artifactsAll scripts, per-version and per-commit logs, and bisect logs are in |
reitblatt
force-pushed
the
update-protobuf
branch
from
June 17, 2026 16:34
a07723e to
b4b508c
Compare
Contributor
Author
The comments + changelog were out of date after the latest iterations. This PR actually pins grpcio to 1.74.0. I'll fix the changelog. |
Contributor
Author
Collapses the platform_machine split (grpcio 1.49.1 on x86_64, 1.53.x elsewhere) into a single pin now that upstream ships wheels for both platforms again. Also bumps typing-extensions to 4.12.2, which grpcio 1.82.1 requires (>=4.12,<5).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
reitblatt
force-pushed
the
update-protobuf
branch
from
July 13, 2026 17:55
b4b508c to
0004923
Compare
Member
|
@reitblatt that would be great, if you can help separate it! Thanks so much! |
Update grpcio/grpcio-tools to 1.74.0/1.71.2, google-cloud-pubsub, and tink to versions compatible with protobuf 5.x. Relax google-cloud-kms, grpcio-health-checking, grpcio-reflection, and grpcio-status from exact pins to floor constraints. Unify the grpcio platform split into a single pin. google-cloud-logging is not bumped here: it (along with google-cloud-secret-manager, google-cloud-appengine-logging, and google-cloud-audit-log) was dropped as an unused Discord-era dependency by roostorg#391 while this branch was in flight, so there's nothing left to upgrade. Regenerate all *_pb2.py files via ./gen-protos.sh. Test plan: ./run-tests.sh
reitblatt
force-pushed
the
update-protobuf
branch
from
July 13, 2026 19:07
0004923 to
b923855
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pyproject.toml (1)
83-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
types-protobufis pinned two major versions behind theprotobufruntime.
types-protobuf==4.24.0.1(line 83, unchanged) is now mismatched withprotobuf==5.29.6(line 47). The 4.x type stubs won't cover protobuf 5.x API changes, which could cause mypy to miss type errors or flag false positives in code that uses protobuf 5.x features. Consider bumpingtypes-protobufto a 5.x-compatible release.Note: mypy excludes
*_pb2*.pyfiles (lines 265-267), so generated code is unaffected, but hand-written code that imports protobuf types directly would be impacted.As per path instructions, this dependency upgrade requires human approval for license and CVE review.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` at line 83, Update the types-protobuf dependency entry to a 5.x-compatible release matching the protobuf==5.29.6 runtime, then request human approval for the required license and CVE review.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pyproject.toml`:
- Line 83: Update the types-protobuf dependency entry to a 5.x-compatible
release matching the protobuf==5.29.6 runtime, then request human approval for
the required license and CVE review.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd1b7f4b-1e99-4871-b92c-c18110151ee6
⛔ Files ignored due to path filters (37)
osprey_rpc/src/osprey/rpc/actions/v1/action_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/action_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/action_types_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/action_types_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/application_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/application_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/captcha_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/captcha_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/channel_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/channel_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/guild_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/guild_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/invite_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/invite_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/metadata_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/metadata_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/user_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/actions/v1/user_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/execution_result_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/execution_result_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/take_data_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/take_data_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/verdicts_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/common/v1/verdicts_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/etcd_watcherd/v1/etcd_watcherd_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/etcd_watcherd/v1/etcd_watcherd_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/bidirectional_stream/v1/service_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/bidirectional_stream/v1/service_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/sync_action/v1/service_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/osprey_coordinator/sync_action/v1/service_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/tests/v1/tests_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/tests/v1/tests_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/v1/options_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/pigeon/v1/options_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/request_caching/v1/service_pb2.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyosprey_rpc/src/osprey/rpc/request_caching/v1/service_pb2_grpc.pyis excluded by!osprey_rpc/src/osprey/rpc/**/*_pb2*.pyuv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
CHANGELOG.mdpyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
Update grpcio-tools to 1.82.1 (to match grpcio, and required by grpcio-tools itself for protobuf 7.x support), google-api-core to 2.31.0, googleapis-common-protos to 1.75.0, and grpc-google-iam-v1 to 0.14.4 — all of which capped protobuf below 7.0 at their prior pins. Also bump types-protobuf to match the new protobuf major version. google-cloud-pubsub, tink, and google-cloud-kms already tolerate protobuf 7.x, so no changes needed there. Regenerate all *_pb2.py files via ./gen-protos.sh. Test plan: ./run-tests.sh (1176 passed, 0 failed, 0 errors) uv run mypy osprey_worker osprey_rpc osprey_async_worker example_plugins
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Depends on #415 (branch rebased on top of it) — grpcio is bumped there separately. Until #415 merges into main, this PR's diff/commit list will include its two commits (
Bump grpcio to 1.82.1,Add CHANGELOG entry for grpcio bump); they'll drop out automatically once #415 lands.On top of that, this PR upgrades protobuf and grpcio-tools, and as a downstream consequence, google-cloud-pubsub and tink, to versions compatible with protobuf 5.x. Relaxes google-cloud-kms, grpcio-health-checking, grpcio-reflection, and grpcio-status from exact pins to floor constraints.
Regenerate all *_pb2.py files via ./gen-protos.sh.
Resolves #316, #317
Test plan:
./run-tests.sh
Checklist
uv run ruff check .passes (no unused imports or other lint errors)uv tool run fawltydeps --check-unused --pyenv .venvpasses (no unused dependencies)CHANGELOG.mdwith my changes, if applicableSummary by CodeRabbit
Summary by CodeRabbit