Deploy preview for hasura-docs ready!

Built with commit f8264cd

https://deploy-preview-3686--hasura-docs.netlify.com

Review app for commit ce6e1bc deployed to Heroku: https://hge-ci-pull-3686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3686-ce6e1bc4

Review app for commit f8264cd deployed to Heroku: https://hge-ci-pull-3686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3686-f8264cd0

Currently we have replace_metadata which is backed by a ReplaceMetadata type (should've been called something like GraphqlEngineMetadata) which sets graphql-engine's metadata to the given metadata.

I'm wondering if it makes sense to have a query like add_metadata which instead of replacing, 'merges' the given metadata into the existing metadata. Is it possible to have add_metadata to perform better than a bulk query of adding tables, relationships etc? If so, we can just specify our internal catalog as a single query instead of using a bulk query like we do currently.

(The implementation in this PR looks like an interface to add_metadata which can only be used internally)

Currently we have replace_metadata which is backed by a ReplaceMetadata type (should've been called something like GraphqlEngineMetadata) which sets graphql-engine's metadata to the given metadata.

I'm wondering if it makes sense to have a query like add_metadata which instead of replacing, 'merges' the given metadata into the existing metadata. Is it possible to have add_metadata to perform better than a bulk query of adding tables, relationships etc? If so, we can just specify our internal catalog as a single query instead of using a bulk query like we do currently.

I considered this, too, and I think it’s a totally viable alternative. I wasn’t sure whether it was better to split out some ReplaceMetadata functionality to do what we want or just do it in code.

Eventually, I decided to just do it in the code because that seemed very easy, and it’s very simple to change later. I figured that what we really want is a more general solution to this that helps CLI migrations, too, and maybe at that point we could go back to sharing the same code path, but I couldn’t figure out exactly what I thought that should look like, so I opted to do the simplest thing that seemed least likely to introduce tech debt.

That said, I’m really not attached to this, and in fact I’d actually love for someone else to make this decision for me. 😄 If you think I should tweak this to keep the .yaml file but go through a ReplaceMetadata-like code path, I’m happy to do that.

By the way, I should add why I didn’t want to implement the AddMetadata API you’re suggesting: I could not figure out how to have good error reporting for it without doing a lot of additional engineering to associate all that information properly. So I wouldn’t want to expose it as a user-facing API without thinking a lot harder about what the semantics ought to be, and at that point it seemed less clearly valuable.

We could still have something that works like that internally that we use for this code path but not actually expose it as a valid RQL query, and we could adapt it into a full API at some point in the future. But I don’t think it makes sense to add that API right now without more thought and discussion.

I could not figure out how to have good error reporting for it without doing a lot of additional engineering to associate all that information properly

Yup, this was one of the concerns that I had.

That said, I’m really not attached to this, and in fact I’d actually love for someone else to make this decision for me

Let's keep this implementation for now, given that it improves the startup time. We should definitely think about the right semantics for an add_metadata like query and in general how to improve the performance of cli migrations.

Review app https://hge-ci-pull-3686.herokuapp.com is deleted