Page MenuHomePhabricator

[services] Upgrading Rust Tonic crate version to 0.8 and Prost to 0.11
ClosedPublic

Authored by max on Oct 3 2022, 7:22 AM.
Tags
None
Referenced Files
F3245898: D5285.id17288.diff
Thu, Nov 14, 8:40 PM
F3245725: D5285.id17874.diff
Thu, Nov 14, 8:14 PM
F3245643: D5285.id19268.diff
Thu, Nov 14, 7:48 PM
F3245245: D5285.diff
Thu, Nov 14, 5:21 PM
Unknown Object (File)
Tue, Nov 12, 12:06 AM
Unknown Object (File)
Tue, Nov 5, 9:32 AM
Unknown Object (File)
Tue, Nov 5, 9:32 AM
Unknown Object (File)
Tue, Nov 5, 9:32 AM

Details

Summary

Update the Rust Tonic crate to use the 0.8 version across all services.
To fix the gRPC generation error the prost should be upgraded as well.
The installation of the protoc compiler was added to the identity service Dockerfile.

Raised at D5264.
Related Linear task: ENG-1949

Test Plan

The services are successfully built.

Diff Detail

Repository
rCOMM Comm
Branch
tonic-bump
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

max held this revision as a draft.

prost version upgrade to 0.11

max retitled this revision from [services] Upgrading Rust Tonic crate version to 0.8 to [services] Upgrading Rust Tonic crate version to 0.8 and Prost to 0.11.Oct 4 2022, 10:35 PM
max edited the summary of this revision. (Show Details)
max edited the test plan for this revision. (Show Details)
max added reviewers: marcin, michal, tomek, varun.

looks like the identity image will need to be updated to install protoc in PATH for it to be found.

max removed a reviewer: tomek.

Making this a Draft until the Identity service failing fix solution decision comes in ENG-1949

Adding installation of the protoc to the identity service Docker image.

max published this revision for review.Dec 8 2022, 8:32 AM
max edited the summary of this revision. (Show Details)
max added 1 blocking reviewer(s): varun.

Eventually, I would like to get the docker images to also use the same protoc as the rest of the project, ubuntu has been on 3.12 for the past ~4 years.

But that's future work, this looks fine.

varun requested changes to this revision.Dec 9 2022, 5:49 AM
varun added inline comments.
services/identity/Dockerfile
3 ↗(On Diff #19250)

Why do we have to add this? Curious why things worked without installing protobuf-compiler when we were on tonic 0.6

This revision now requires changes to proceed.Dec 9 2022, 5:49 AM
max marked an inline comment as done.
max added inline comments.
services/identity/Dockerfile
3 ↗(On Diff #19250)

Why do we have to add this? Curious why things worked without installing protobuf-compiler when we were on tonic 0.6

It's related to the recent prost version. Since 0.11 it requires to protoc to be available.

Got it, and what’s in the lists folder we’re deleting in that command?

This revision is now accepted and ready to land.Dec 9 2022, 5:59 AM
In D5285#174742, @varun wrote:

Got it, and what’s in the lists folder we’re deleting in that command?

We are removing the cache from the apt-get update because we don't need it anymore, but it took up a lot of disk space.

This revision now requires review to proceed.Dec 9 2022, 6:03 AM
ashoat added inline comments.
services/identity/Dockerfile
4 ↗(On Diff #19250)

Can you indent this line so it's clear that it's associated with the above line?

This revision is now accepted and ready to land.Dec 9 2022, 7:02 AM
max marked an inline comment as done.

Adding of indent.

services/identity/Dockerfile
4 ↗(On Diff #19250)

Can you indent this line so it's clear that it's associated with the above line?

Sure! Done.