apollographql / rover

The CLI for Apollo GraphOS
https://rover.apollo.dev
Other
408 stars 85 forks source link

feat: `rover supergraph dev` #1258

Closed EverlastingBugstopper closed 1 year ago

EverlastingBugstopper commented 2 years ago

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 commented 2 years ago

a few issues with this:

smyrick commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 😁

lennyburdette commented 2 years ago

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:

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

danpayne17 commented 1 year ago

@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