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

Integrate async-graphql #822

Closed
0xmovses opened this issue Apr 27, 2023 · 1 comment · Fixed by #877
Closed

Integrate async-graphql #822

0xmovses opened this issue Apr 27, 2023 · 1 comment · Fixed by #877
Assignees
Labels
fuel-explorer This PR is directly related to the block explorer
Milestone

Comments

@0xmovses
Copy link
Contributor

0xmovses commented Apr 27, 2023

Context and History 📚

This originally came up in a comment on a PR here. Some discussion has been had about the pros and cons of moving away from our own fuel-indexer-graphql-parser package and to async-graphql.

The original fuel graphql api not maintained by us made use a forked version of graphql-parser. We then added this code into our repo and we make heavy use of its types and other functionalities. This was merged in to master in this PR.

The use of the async-graphql crate came into play when we started supporting the GraphQL playground, we needed the crate in order to provide this functionality. See the merged PR for more context.

In its current state, we are using both these crates.

Why move to async-graphql? 🤔

• Async-graphql provides a lot of functionality out of the box, such as docs and schemas within the playground, multiple queries in a single request and probably much more. Full list (copied from their readme)

  • Static and dynamic schemas are fully supported
  • Fully supports async/await
  • Type safety
  • Rustfmt friendly (Procedural Macro)
  • Custom scalars
  • Minimal overhead
  • Easy integration (poem, axum, actix-web, tide, warp, rocket ...)
  • Upload files (Multipart request)
  • Subscriptions (WebSocket transport)
  • Custom extensions
  • Error extensions
  • Limit query complexity/depth
  • Batch queries
  • Apollo Persisted Queries
  • Apollo Tracing extension
  • Apollo Federation(v2)

Currently we have to hand-roll a lot of this functionality as we are using the fuel-indexer-graphql-parser types.

• By utilising the full potential of async-graphql we would reduce the size of our binaries, as we would perhaps not need to use fuel-indexer-graphql-parser.

• Probably most of the features public devs will want from the api, will be easily importable using the package.

Why not to move to async-graphql? 🔕

• The main reason we use our own types is because we process custom, user generated schemas not known at compile time. Furthermore, these schemas do not support all graphql types, because of our codegen.

• We stay in control of our own parser types and functionality, no reliance on a package for the critical piece.

The main point -> Supposedly async-graphql supports dynamic schemas see here but then @deekerno also found this issue, which suggests the opposite. This is unclear for us currently.

• We need to test this dynamic schema functionality, if it can support our needs, then this would be a very powerful addition, and would mean we would need to hand craft much of the graphql functionality already provided by async-graphql

After the questions in this issue are answered, and the unknowns made known, we can come up with a plan about how to deprecate our old graphq-parser code (if we indeed choose to).

Sub-issues (feel free to add more) 🏁

Dynamic Schemas

Feel free to discuss and share your thoughts @ra0x3 @deekerno

@0xmovses 0xmovses changed the title Test out async-graphql capabilities. Integrate async-graphql Apr 27, 2023
@deekerno
Copy link
Contributor

deekerno commented Apr 28, 2023

@rvmelkonian, @ra0x3: I've listed some of my thoughts here.

what we need

For query execution:

  • Ability to store and restore GraphQL schemas from the DB
    • our entire querying ability is quite literally built upon the ability to load schemas at runtime
  • Parse and validate queries
  • Ability to extend our own argument parsing
  • Has to be able to avoid N+1 query problem
  • support for numbers bigger than u64
  • Needs to be able to support programmatic transformation of query into SQL

For schema creation:

  • Need to be able to parse the schema document to create the corresponding DB tables
    • We would need to ensure that all of our current definitions have an equivalent in async-graphql-parser (which shouldn't be hard if it's spec-compliant)

For macros:

  • Need to ensure that all of the current defintions have an equivalent

what async-graphql can do

Of these, the following items are of immediate note:

  • dynamic schemas
  • type safety
  • custom scalars
  • subscriptions
  • limit query complexity
  • federation

dynamic schemas

async-graphql supports dynamic schemas; this example shows how a schema would be created on the fly
- https://github.com/async-graphql/examples/blob/7eee23ad4e005ac180df8111e6940cebb7c1f52f/models/dynamic-starwars/src/model.rs
- https://github.com/async-graphql/examples/blob/7eee23ad4e005ac180df8111e6940cebb7c1f52f/models/dynamic-starwars/src/lib.rs

Due to our requirement of supporting multiple schemas at runtime and querying a database for records, we need to ensure that we make as few queries as possible. If anything, we would probably make a general schema type that contains a functions that just queries the DB after parsing the data through fuel-indexer-graphql but still provides the ability for introspection.

type safety

If we were using dynamic schemas as intended, this would be more of a benefit for us as we could leverage the type system to ensure correctness instead of manually checking the SQL and its result set. However, since we probably won't be able to use it in that way, we may just be able to use it to ensure that results conform to the necessary structure.

custom scalars

async-graphql allows for the creation of custom scalars; see https://async-graphql.github.io/async-graphql/en/custom_scalars.html. We may be able to use this functionality to implement the ability to properly support large integer types (u128, u256, etc.).

subscriptions

Using GraphQL subscriptions would be an easy win for real-time functionality. My main concern here is how to do we return a stream of data from the database and ensure that we're not sending the same data over and over. Also, would it play nice with paginated queries? Is pagination even compatible with the streaming paradigm?

limit query depth and complexity

async-graphql would allow us to limit query depth and complexity with a few short lines; see https://async-graphql.github.io/async-graphql/en/depth_and_complexity.html.

federation

async-graphql supports federation.

initial plan

Keep in mind that this is all off the top of the dome and needs to be subjected to rigourous testing, but here's an initial plan:

  • On the schema creation and macro sides: replace fuel-indexer-graphql-parser with async-graphql-parser. Both crates are fully spec-compliant, so the effort here should just require finding the equivalent types and adjusting the plumbing around them. The potential benefit of this is two-fold:
    • Remove the burden of maintaining all of the extra code in the repo
    • Should allow for us to definite custom scalars, which enables use to add more types in the future
  • On the query side:
    • Use dynamic schemas to build the type structure for a schema so that it can be used for introspection in a GraphQL playground
    • Enforce query complexity limits at the query parsing level; this would allow us to return an error message earlier than the GQL->SQL transformation step
    • Create separate API querying functionality in which a subscription is created over a websocket connection and then SQLx streams in the data from a query until the subscription is terminated

@deekerno deekerno self-assigned this May 1, 2023
@ra0x3 ra0x3 added this to the Beta-4.5 milestone May 1, 2023
@ra0x3 ra0x3 added the fuel-explorer This PR is directly related to the block explorer label May 8, 2023
@deekerno deekerno linked a pull request May 10, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuel-explorer This PR is directly related to the block explorer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants