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

Bump prost, tonic, and the generated files #29

Merged
merged 6 commits into from
Apr 20, 2023
Merged

Bump prost, tonic, and the generated files #29

merged 6 commits into from
Apr 20, 2023

Conversation

jneem
Copy link
Collaborator

@jneem jneem commented Apr 19, 2023

This is the main breaking-change part of #20, so I figured it was worth separating out.

@jneem
Copy link
Collaborator Author

jneem commented Apr 19, 2023

I'm pretty stumped by the CI failure here. I can't reproduce it locally, and clearly axum 0.6 exists...

@rnarubin
Copy link
Collaborator

tonic 0.8 depends on axum 0.5, while tonic 0.9 uses axum 0.6. The lock file has tonic 0.8 and axum 0.6, which are incompatible -- maybe a result of some mixups while iterating on versions?

I think a cargo update could resolve this. Though if we're making the change anyway, bumping to tonic 0.9 is probably a good idea

@rnarubin rnarubin self-requested a review April 19, 2023 17:52
@jneem
Copy link
Collaborator Author

jneem commented Apr 19, 2023

Hm, tonic 0.9 is causing a failed test ("Error, message length too large: found 10000520 bytes, the limit is: 4194304 bytes"), but also from here it seems like tonic 0.8 should use axum 0.6.

@rnarubin
Copy link
Collaborator

oh tonic 0.8.2 uses axum 0.5, 0.8.3 uses 0.6. maybe that was the mixup?

tonic 0.9 is causing a failed test ("Error, message length too large: found 10000520 bytes, the limit is: 4194304 bytes")

which test?

@jneem
Copy link
Collaborator Author

jneem commented Apr 19, 2023

pubsub::publish_sink::test::message_responses_in_order. The full error is

Error: Status { code: OutOfRange, message: "Error, message length too large: found 10000520 bytes, the limit is: 4194304 bytes", metadata: MetadataMap { headers: {"content-type": "application/grpc", "grpc-encoding": "identity", "grpc-accept-encoding": "gzip"} }, source: None }

@jneem
Copy link
Collaborator Author

jneem commented Apr 19, 2023

It's probably this.

@rnarubin
Copy link
Collaborator

ah yeah. others have run into this, looks like 10MB is the limit pubsub set GoogleCloudPlatform/pubsub#164

@jneem
Copy link
Collaborator Author

jneem commented Apr 20, 2023

Ok, tonic is bumped to 0.9, and I put the pubsub size at 10MB. I made the bigtable size unlimited because I couldn't find a documented limit. (Pre-tonic 0.9 it was unlimited anyway, so this keeps the old behavior.)

Tonic-build 0.9 seems to assume edition 2021, so I also bumped the edition (which required no other changes). Finally, I bumped the MSRV to 1.63 because time insisted.

Copy link
Collaborator

@rnarubin rnarubin left a comment

Choose a reason for hiding this comment

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

👍

@rnarubin rnarubin merged commit ae51cd6 into master Apr 20, 2023
@rnarubin rnarubin deleted the prost-bump branch April 20, 2023 20:50
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