RFC: simplify subgraph getting started and federation #10303
Conversation
|
|
nullxone
force-pushed
the
ak/rfc/20240620-simplify-subgraph-getting-started-and-federation
branch
from
41fe72a
to
7ab3fa5
Compare
|
|
||
| ```bash | ||
| # creates a subgraph folder and subgraph.yaml file | ||
| ddn subgraph init app |
There was a problem hiding this comment.
Assuming this command is run in a DDN project directory (where we can locate hasura.yaml)? Then we create the app directory at the root of the project? It would not make a lot of sense if the directory in created in the CWD. We should also think about the variant for federation use case, where we init a standalone subgraph (thus creating hasura.yaml, supergraph.local.yaml, the globals directory, engine, docker-compose files etc)
There was a problem hiding this comment.
Yes, let's keep the existing behaviour of putting next to hasura.yaml.
Let's also introduce a --dir flag to explicitly give the directory like we do with connectors.
|
|
||
| # add subgraph.yaml to the supergraph.yaml file and create an env file for the env | ||
| ddn supergraph add subgraph --supergraph supergraph.local.yaml --subgraph app/subgraph.yaml | ||
| touch app/.env.app.local |
There was a problem hiding this comment.
From a primitives PoV, touch to create a .env file makes sense. But this is gonna be different for UNIX and powershell. Do during subgraph init, can we by default initialize a .env.app.local file (and add it to the subgraph.yaml as well)? The rationale is we init a subgrph, do local dev and then go to cloud, so a local .env file is absolutely needed here, so why not create it?
There was a problem hiding this comment.
ddn connector-link add takes a --configure-host flag. Add a --env-file flag here as well. Creates the env file if not present.
Now we can remove the touch command.
There was a problem hiding this comment.
The command should be ddn subgraph add
There was a problem hiding this comment.
Why don't we initialize a subgraph.cloud.yaml and subgraph.local.yaml` when we initialize a subgraph along with the corresponding .env files? The files will be the same except for the .env file.
The advantages of doing so are:
- We generate local and cloud files for supergraph and connector. We also have to generate local and cloud file for the globals subgraph.yaml. Doing the same when initializing a subgraph would keep the behaviour consistent with everything else we are doing.
- It would make the commands much more simpler as users don't need to pass a
--env-fileflag. Advanced users can still do it if they want but for simple use cases and getting started, not having to pass an additional flag would keep things simpler and easier to grasp. The fact that it is more consistent as mentioned in (1) would also make it more easier to grasp.
The only drawback is that we'll have 2 files that are the same except for the .env file. However since this is how it's done in all other cases and since this will eliminate the need to add an extra flag in all the commands, this seems like the more simpler approach.
There was a problem hiding this comment.
The command should be ddn subgraph add
IMO, this sounds like we are adding something to the subgraph. I feel like ddn supergraph add subgraph as in the original makes more sense although it's more verbose. Or were you talking about something else?
There was a problem hiding this comment.
Let's go with
- single subgraph.yaml
- scaffold both .env.app.local and .env.app.cloud
This does not introduce additional subgraph.yaml files with no differences, but also makes the model clear to the user.
There was a problem hiding this comment.
ddn subgraph add --supergraph
is along the lines of
ddn model add --subgraph
and so is consistent
The alternative here could be
ddn supergraph subgraph add
Which is more verbose and doesn't really add much value. We've essentially raised subgraph to a top level subcommand exactly like models.
| touch app/.env.app.local | ||
|
|
||
| # create the supergraph build | ||
| ddn supergraph build --supergraph supergraph.yaml --env-file app/.env.app.local |
There was a problem hiding this comment.
In supergraph build, there could be multiple subgraphs and each can have its own env file. With this command, we cannot determine, which env file belongs to which subgraph.yaml. We might need to do something like the prefixing we did for the -e flag like --env-file <subgraph-Name>=<path-to-env-file>. Looks ugly though
There was a problem hiding this comment.
You're right. Can we do --env-file <subgraph>:<env file>.
Can do --env <subgraph>:<key>=<value> for consistency.
|
|
||
| # deploy to cloud | ||
| touch app/.env.app.cloud | ||
| ddn connector build create --connector connector.yaml --target-env-file app/.env.app.cloud |
There was a problem hiding this comment.
We might not need a touch here. If the command is like ddn connector build create --connector mydb/connector.yaml --target-subgraph app/subgraph.yaml --target-connector-link mypg --target-env-file app/.env.app.cloud . If the --target-env-file is not provided, we use the envFile specified in subgraph.yaml. If the flag is provided, we create the file if it does not exist and write it there
There was a problem hiding this comment.
Agree, can do without touch. Makes for better cross platform docs as well.
| What this is about: on introducing federation, we realize that the primitive is the subgraph not the supergraph. The following commands are updated to consider the subgraph.yaml file instead of specifying a supergraph.yaml file and the subgraph by name. | ||
|
|
||
| ```bash | ||
| ddn connector init mydb --subgraph app/subgraph.yaml --hub-connector hasura/postgres |
There was a problem hiding this comment.
These commands make sense. This would mean that --supergraph flag goes away in these commands. This is a breaking change. I guess we should be ok with that. Just calling it out.
There was a problem hiding this comment.
Yes, this is breaking. Confirmed.
There was a problem hiding this comment.
Do we want to/ would it be possible to make it backwards compatible by allowing --subgraph to accept either name or filepath. That way existing users can continue to use the command and new users will be able to use the new approach. We can even show a deprecation notice if someone passes subgraph name in flag and maybe even remove the behaviour a few versions down the line.
Not sure if any of this is worth it but just putting it out here for thought.
| touch app/.env.app.local | ||
|
|
||
| # create the supergraph build | ||
| ddn supergraph build --supergraph supergraph.yaml --env-file app/.env.app.local |
There was a problem hiding this comment.
You're right. Can we do --env-file <subgraph>:<env file>.
Can do --env <subgraph>:<key>=<value> for consistency.
|
|
||
| # deploy to cloud | ||
| touch app/.env.app.cloud | ||
| ddn connector build create --connector connector.yaml --target-env-file app/.env.app.cloud |
There was a problem hiding this comment.
Agree, can do without touch. Makes for better cross platform docs as well.
| What this is about: on introducing federation, we realize that the primitive is the subgraph not the supergraph. The following commands are updated to consider the subgraph.yaml file instead of specifying a supergraph.yaml file and the subgraph by name. | ||
|
|
||
| ```bash | ||
| ddn connector init mydb --subgraph app/subgraph.yaml --hub-connector hasura/postgres |
There was a problem hiding this comment.
Yes, this is breaking. Confirmed.
|
|
||
| # add subgraph.yaml to the supergraph.yaml file and create an env file for the env | ||
| ddn supergraph add subgraph --supergraph supergraph.local.yaml --subgraph app/subgraph.yaml | ||
| touch app/.env.app.local |
There was a problem hiding this comment.
The command should be ddn subgraph add
|
|
||
| ddn model add --subgraph app/subgraph.yaml --connector-link mydb --name Album | ||
|
|
||
| ddn connector build get --connector-name mypg --subgraph-name app --project proj-1234 |
There was a problem hiding this comment.
Why don't we use --subgraph here also and we could get the subgraph name from the definition?
I get that the subgraph mentioned here is the subgraph in the cloud project. If so, then should the flag be --project-subgraph so that it will be similar to the command related to the same concept ddn project subgraph.
Simplify subgraph getting started and federation
Continuing with the theme of designing around primitives, and optimising for a user getting started going from scratch to an API with only CLI commands, the following changes are related to subgraphs.
Rendered: https://github.com/hasura/graphql-engine/blob/ak/rfc/20240620-simplify-subgraph-getting-started-and-federation/v3/rfcs/dx/20240620-simplify-subgraph-getting-started-and-federation.md