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

feat: implement grpc_gateway api client #222

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Conversation

dmccartney
Copy link
Collaborator

@dmccartney dmccartney commented Sep 11, 2023

This begins the implementation of the wasm-friendly grpc-gateway API client.
It continues where #217 left off.

  • split xmtp_networking into xmtp_api_grpc and xmtp_api_grpc_gateway
  • setup CI for xmtp_api_grpc_gateway
  • implement grpc-gateway Query
  • implement grpc-gateway Publish
  • implement grpc-gateway BatchQuery

Future PR

  • implement grpc-gateway Subscribe
  • xmtp surgery to make storage layer swappable
  • basic sample app (to feel concurrency/perf beyond automated tests)

@dmccartney dmccartney force-pushed the daniel-api-gateway branch 7 times, most recently from 35ff5ac to 8948a20 Compare September 14, 2023 20:53
Comment on lines +56 to +57
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what is the reasoning behind this? We need Send on non-wasm, but we don't need it on wasm?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, that's right -- I know, it's an eyesore :( (more info here).

More generally, I don't love the look of this yet. But I was thinking to revisit it after / as I sort out how to implement the subscribe() call since I suspect it will have the gnarlier concurrency issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's totally fine, makes sense to prioritize other things! The surface area of this work is so large. Just wanted to understand it for future reference

@dmccartney dmccartney force-pushed the daniel-api-gateway branch 2 times, most recently from d97e9b8 to 4191f08 Compare September 14, 2023 22:44
@dmccartney dmccartney force-pushed the daniel-api-gateway branch 6 times, most recently from 81e6238 to 4d448d2 Compare September 28, 2023 15:41
@dmccartney dmccartney marked this pull request as ready for review September 28, 2023 15:45
@dmccartney dmccartney requested a review from jhaaaa as a code owner September 28, 2023 15:45
@dmccartney dmccartney requested review from a team and nplasterer September 28, 2023 15:45
QueryResponse, SubscribeRequest,
};

// TODO: consider moving these (and other address const) into `xmtp_proto`
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@dmccartney dmccartney merged commit 1b05872 into main Sep 29, 2023
16 checks passed
@dmccartney dmccartney deleted the daniel-api-gateway branch September 29, 2023 19:19
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.

4 participants