-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
rest: add support for protobuf to remaining endpoints that support a binary content type #20096
Conversation
Remove the ListFullCheckpoint api as its a bit too heavy-weight and instead we'll look to introduce a streaming api in the future.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
…saction endpoints
After talking with @wlmyng I'll add a commit on top of this to have the client request BCS data till this rolls out to all networks |
Revert the client change to request protobuf as a response type, and instead request bcs, because the support for protobuf hasn't rolled out to other networks yet.
.into_inner(); | ||
proto.try_into().map_err(Into::into) | ||
self.inner.bcs(request).await.map(Response::into_inner) | ||
// let proto = self |
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.
Nit: comment
@@ -337,6 +337,36 @@ impl TryFrom<&UserSignature> for sui_types::signature::GenericSignature { | |||
} | |||
} | |||
|
|||
// |
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.
Is this file translations between various types
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.
I think the 2 step rollout makes sense i.e. server followed by client
Yeah, that and while I think these protos are a good start I think it may be good to sit on them for a short period of time (a week or two) to make sure there aren't any other changes we want to make before we can't trivially tweak them. |
Description
Describe the changes or additions included in this PR.
Test plan
How did you test the new or updated feature?
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.