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: rover supergraph dev #1258

Closed
EverlastingBugstopper opened this issue Aug 25, 2022 · 18 comments · Fixed by #1627 or #1642
Closed

feat: rover supergraph dev #1258

EverlastingBugstopper opened this issue Aug 25, 2022 · 18 comments · Fixed by #1627 or #1642
Labels
feature 🎉 new commands, flags, functionality, and improved error messages needs design ✏️

Comments

@EverlastingBugstopper
Copy link
Contributor

rover dev is great and will be even better when you can extend a remote router. right now though it's a bit cumbersome if you want to start up 12 different subgraphs locally. we should introduce a rover supergraph dev command that reads from a supergraph.yaml file.

the plumbing should be similar to rover dev except we'll need to add a polling mechanism for rover subgraph fetch invocations.

@EverlastingBugstopper
Copy link
Contributor Author

a few issues with this:

  • live reloading a supergraph.yaml that pulls from Apollo Studio is likely a non-starter because we'd probably DDoS ourselves. we'd need to only poll introspection/the file system for schemas and have a warning printed at the start of the command that says you need to restart your server to pick up incoming changes from the registry.
  • we want to move away from supergraph.yaml and more towards the registry, so introducing this pattern is likely a step in the wrong direction and we should wait until you can extend a remote router with a local subgraph

@smyrick
Copy link
Member

smyrick commented Aug 26, 2022

We might want to rethink "moving away from supergraph.yml" because fundamentally that is what this feature is going to require. We could still have a local file but maybe we want to start phasing out the ability to run local composition, or require a graph id, or we could move composition to an API call but that gets into scale issues again.

ie

Run the mygraph@prod with all the same URLs but override the products subgraph with this schema/url

#supergraphl.yml
graphref: mygraph@prod

subgraph_overrides:
  - name: products
     url: http://localhost:4000

@smyrick
Copy link
Member

smyrick commented Aug 26, 2022

If we are not going to allow fetch from an introspection we could have subgraphs push to new temp variant in Studio, any make sure Studio can handle running that many variants. This would have to be thought of the same way as GitHub handles branches today

@EverlastingBugstopper
Copy link
Contributor Author

for folks who want this - we are planning to extend rover dev's functionality to allow you to extend a remote router with one or more local subgraphs. if that use case does not work for your plans, we'd love to hear about it.

@dbanty
Copy link
Contributor

dbanty commented Sep 15, 2022

I would love to update async-graphql's federation example to use rover/router instead of Apollo Server + Gateway. It would be much easier if all the subgraphs could be defined in a single file, similar to how you can define them in JavaScript for Gateway.

The ability to run a small supergraph locally-only for debugging/hacking a PoC is something I do all the time. Making it a few steps easier would be great DX 😁

@EverlastingBugstopper EverlastingBugstopper added feature 🎉 new commands, flags, functionality, and improved error messages needs design ✏️ labels Sep 15, 2022
@lennyburdette
Copy link
Contributor

I don't personally care if it's local or remote, and half the time I don't even care if a router is actually running. I mostly want the ability to test out composition concepts and get real-time feedback. I often need to:

  • test out a new composition feature that works across multiple subgraphs like @composeDirective
  • demonstrate composition to customers and show the resulting errors and hints
  • debug composition errors for customers by pulling down their subgraphs and swapping in their proposed changes
  • see query plans for customers (i guess this could work with the router's new "expose query plan" functionality)

as an SA/customer advocate, my needs are not the same as our customer's needs so i don't necessarily expect rover to be the tool for this. but it might be really cool if it was.

@patrick91
Copy link
Contributor

We might want to rethink "moving away from supergraph.yml" because fundamentally that is what this feature is going to require. We could still have a local file but maybe we want to start phasing out the ability to run local composition, or require a graph id, or we could move composition to an API call but that gets into scale issues again.

I wonder if this file could also be a way to "deploy" a cloud supergraph, like a terraform file.

@westhechiang
Copy link

for folks who want this - we are planning to extend rover dev's functionality to allow you to extend a remote router with one or more local subgraphs. if that use case does not work for your plans, we'd love to hear about it.

This would be a great feature since our developers currently need to run rover dev for each subgraph despite only needing to work on 1 of the subgraphs.

@BrennanRoberts
Copy link

BrennanRoberts commented Jun 8, 2023

Even just being able to (1) start a rover dev session from a router config file pointed to subgraph defaults and then (2) overwrite existing subgraphs on the active rover dev session would be good start towards a more sophisticated implementation.

  1. Start a local router session pointed at all the default (likely hosted) versions of all your subgraphs
  2. Start foo-subgraph locally at localhost:4000
  3. Run rover dev --name foo-subgraph --url http://localhost:4000/, causing the existing session to replace the old foo-subgraph with the new one

Today, step 3 fails with error: subgraph with name 'foo-subgraph' is already running in this `rover dev` session.

@dbanty
Copy link
Contributor

dbanty commented Jun 9, 2023

Start a local router session pointed at all the default (likely hosted) versions of all your subgraphs

How would this initial session be started? Is this using a local file (if so, which format) to point at the deployed subgraphs, or loading from GraphOS via Graph Ref?

@danpayne17
Copy link

I'll chime in with our use cases here at Charter:

  • Run a subset of subgraphs (say 2 or 3 of the 40+ we have) locally in order to debug an issue. We'll need to use our standard router config and have that locally running router connect to a few locally running subgraphs. Would be great to be able to specify that ever changing subset of subgraphs in a file
  • Run a subset of subgraphs that are a mix of locally running and hosted. For hosted subgraphs, we connect to them over a locally running Kubernetes proxy.
  • Run a subset of subgraph but all of them are hosted and a locally running router would need to connect to them over a locally running Kubernetes proxy

The way we handled this with the gateway was to have a custom file that contained a list of all of our subgraphs and for each one it would have a set of properties:

Then when our gateway starts up we can dynamically construct a serviceList by reading this config file. Makes it pretty easy for the gateway to know what to compose from. Also makes it easy to include/exclude various subgraphs by using the mode parameter by setting a subgraph to disabled.

Something similar with the router would be great where we could have a set list of subgraphs and be able to specify which to include in the composition for a given session and what the routing URL will be for each depending on whether its hosted locally or remotely.

@dbanty
Copy link
Contributor

dbanty commented Jun 9, 2023

Thanks for all that info @danpayne17! The current implementation being considered in #1627 uses the existing supergraph.yaml syntax (used by rover supergraph compose) which looks like this:

subgraphs:
  products:
    routing_url: http://localhost:4001
    schema:
      subgraph_url: http://localhost:4001
  users:
    routing_url: http://localhost:4002
    schema:
      subgraph_url: http://localhost:4002

In theory you could comment out the line for the URL you weren't using and swap them like that (instead of having an explicit mode). The biggest challenge I see is that the routing URL is separate from the introspection URL, which often means duplicating. We probably want a future enhancement which will allow a shorter form that omits one of those URLs and assumes they are the same.

Would a super compact version of this that just looks like:

subgraphs:
  account:  http://localhost:4001 # local
  # account: http://localhost:8001/api/v1/namespaces/cnet/services/graphql-account-service:8080/proxy/graph # proxy
  # user:  http://localhost:4002 # local
  user: http://localhost:8001/api/v1/namespaces/cnet/services/graphql-user-service:8080/proxy/graph # proxy

Where you comment in & out the appropriate line work for you? Would you need to be able to add headers to introspection requests?

@dbanty
Copy link
Contributor

dbanty commented Jun 9, 2023

Another option I have considered is using the Router config's override_subgraph_url field in place of the supergraph.yaml file. Not sure that people will want to be editing their router config, though 🤔

@danpayne17
Copy link

@dbanty Yeah, I like that compact version with the implied subgraph_url. We don't require any headers for introspection requests, so that wouldn't be an issue.

Commenting lines in and out for a given debug session can work, of course, but something cleaner might be a subgraphs.<subgraph>.disabled: <true | false>?

@BrennanRoberts
Copy link

BrennanRoberts commented Jun 12, 2023

@danpayne17's use cases align with ours 👍

In theory you could comment out the line for the URL you weren't using and swap them like that...

Definitely a "nice to have", but allowing subgraphs to be overwritten in a session would avoid the need to decide and declare your overrides in advance and seems in line with the design goal of "Think plug-n-play USB devices but with your GraphQL APIs".

@dbanty
Copy link
Contributor

dbanty commented Jun 12, 2023

Another thought, and let me know if this would work @danpayne17, is to allow providing a list of URIs which Rover would try until it finds one that works. So imagine you had a setup like this:

subgraphs:
  account:
    - http://localhost:3000  # If running source directly
    - http://localhost:8001/api/v1/namespaces/cnet/services/graphql-account-service:8080/proxy/graph  # If running a local k8s cluster
    - https://accounts.dev.myapi.tech  # Fall back to hosted dev environment

Then you wouldn't need to change anything between runs, whatever is accessible is what Rover would use 🤔

@dbanty
Copy link
Contributor

dbanty commented Jun 12, 2023

Created #1633 for subgraph overriding and #1634 for a shorter form of the supergraph.yaml file.

@danpayne17
Copy link

@dbanty love that idea where we provide a list of URLs and it cascades through them until it finds one that works. That would great ease the experience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎉 new commands, flags, functionality, and improved error messages needs design ✏️
Projects
None yet
8 participants