Skip to content
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

chore: 🧃 upgrade to tonic@0.11.0 #4400

Open
cratelyn opened this issue May 16, 2024 · 0 comments
Open

chore: 🧃 upgrade to tonic@0.11.0 #4400

cratelyn opened this issue May 16, 2024 · 0 comments
Labels
A-tooling Area: developer tooling for building Penumbra itself C-chore Codebase maintenance that doesn't fix bugs or add features, and isn't urgent or blocking. _P-medium Medium priority _P-V2 Priority: after mainnet

Comments

@cratelyn
Copy link
Contributor

cratelyn commented May 16, 2024

relates to:

we observed some issues related to gRPC reflection in #4392, and subsequently opened a patch upstream to add the v1 version of the server reflection facilities.

tonic has since released a 0.11.0 version. once that patch is released, we'll need to bump our dependency on tonic to pull in those changes. doing so will involve upgrading from 0.10.2 to 0.11.0, which we should ideally handle first.

@cratelyn cratelyn added A-tooling Area: developer tooling for building Penumbra itself C-chore Codebase maintenance that doesn't fix bugs or add features, and isn't urgent or blocking. labels May 16, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Penumbra May 16, 2024
@cratelyn cratelyn self-assigned this May 16, 2024
@github-actions github-actions bot added the needs-refinement unclear, incomplete, or stub issue that needs work label May 16, 2024
cratelyn added a commit that referenced this issue May 20, 2024
fixes #4392.

in #4392, we found an issue with gRPC reflection when recent versions of
`grpcurl` were used to list supported endpoints.

this was tracked down and fixed in #hyperium/tonic#1701, in
collaboration with @conorsch. that pr targets the more recent 0.11 tonic
release however, which we are not yet using.

eventually we should upgrade to tonic 0.11 (see #4400). as a short-term
measure, but we want to unblock engineers from Skip working on an
integration with Penumbra. this commit patches the `tonic` dependencies
used to point to a temporary fork of the `tonic` repo, where i have
backported that patch (and some ci-related fixes) to the 0.10 release.

see also:
* hyperium/tonic#1701
* penumbra-zone/tonic#1
* penumbra-zone/tonic#2
* #4392
* #4400

---

checking that all transitive dependencies point to the patched version:

```
; cargo tree | grep tonic
│   └── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic-reflection v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7)
│   │   └── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic-web v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-reflection v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-web v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-reflection v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-web v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
```
conorsch pushed a commit that referenced this issue May 21, 2024
fixes #4392.

in #4392, we found an issue with gRPC reflection when recent versions of
`grpcurl` were used to list supported endpoints.

this was tracked down and fixed in #hyperium/tonic#1701, in
collaboration with @conorsch. that pr targets the more recent 0.11 tonic
release however, which we are not yet using.

eventually we should upgrade to tonic 0.11 (see #4400). as a short-term
measure, but we want to unblock engineers from Skip working on an
integration with Penumbra. this commit patches the `tonic` dependencies
used to point to a temporary fork of the `tonic` repo, where i have
backported that patch (and some ci-related fixes) to the 0.10 release.

see also:
* hyperium/tonic#1701
* penumbra-zone/tonic#1
* penumbra-zone/tonic#2
* #4392
* #4400

---

checking that all transitive dependencies point to the patched version:

```
; cargo tree | grep tonic
│   └── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic-reflection v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7)
│   │   └── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic-web v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7)
│   │   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
│   ├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-reflection v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-web v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-reflection v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic-web v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
├── tonic v0.10.2 (https://github.com/penumbra-zone/tonic.git?tag=v0.10.3-penumbra#db355dd7) (*)
```
conorsch added a commit that referenced this issue May 21, 2024
Refs #4392. We want to ensure that server reflection remains
working, despite changes to the tonic dependencies (#4400)
and proto compiling logic #4422. While the most effective test
is exercising all these protos regularly, we currently lack
solid coverage, so this superficial integration test is a sanity-check
to save time versus building locally and inspecting the output manually.
@aubrika aubrika removed the needs-refinement unclear, incomplete, or stub issue that needs work label May 22, 2024
conorsch added a commit that referenced this issue May 23, 2024
Refs #4392. We want to ensure that server reflection remains
working, despite changes to the tonic dependencies (#4400)
and proto compiling logic #4422. While the most effective test
is exercising all these protos regularly, we currently lack
solid coverage, so this superficial integration test is a sanity-check
to save time versus building locally and inspecting the output manually.
conorsch added a commit that referenced this issue May 23, 2024
Refs #4392. We want to ensure that server reflection remains
working, despite changes to the tonic dependencies (#4400)
and proto compiling logic #4422. While the most effective test
is exercising all these protos regularly, we currently lack
solid coverage, so this superficial integration test is a sanity-check
to save time versus building locally and inspecting the output manually.
conorsch added a commit that referenced this issue May 23, 2024
Refs #4392. We want to ensure that server reflection remains
working, despite changes to the tonic dependencies (#4400)
and proto compiling logic #4422. While the most effective test
is exercising all these protos regularly, we currently lack
solid coverage, so this superficial integration test is a sanity-check
to save time versus building locally and inspecting the output manually.
conorsch added a commit that referenced this issue May 23, 2024
Refs #4392. We want to ensure that server reflection remains
working, despite changes to the tonic dependencies (#4400)
and proto compiling logic #4422. While the most effective test
is exercising all these protos regularly, we currently lack
solid coverage, so this superficial integration test is a sanity-check
to save time versus building locally and inspecting the output manually.
conorsch added a commit that referenced this issue May 23, 2024
Refs #4392. We want to ensure that server reflection remains
working, despite changes to the tonic dependencies (#4400)
and proto compiling logic #4422. While the most effective test
is exercising all these protos regularly, we currently lack
solid coverage, so this superficial integration test is a sanity-check
to save time versus building locally and inspecting the output manually.
@aubrika aubrika added _P-medium Medium priority _P-V2 Priority: after mainnet labels May 29, 2024
@cratelyn cratelyn removed their assignment Jun 6, 2024
@cratelyn cratelyn moved this from Backlog to Todo in Penumbra Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tooling Area: developer tooling for building Penumbra itself C-chore Codebase maintenance that doesn't fix bugs or add features, and isn't urgent or blocking. _P-medium Medium priority _P-V2 Priority: after mainnet
Projects
Status: Todo
Development

No branches or pull requests

2 participants