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

Optimize rover dev --graph-ref by fetching all subgraph data at once #1985

Closed
wants to merge 3 commits into from

Conversation

dotdat
Copy link
Contributor

@dotdat dotdat commented Jul 15, 2024

No description provided.

@jonathanrainer jonathanrainer added this to the vNext milestone Jul 18, 2024
jonathanrainer added a commit that referenced this pull request Jul 25, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
jonathanrainer added a commit that referenced this pull request Jul 25, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
jonathanrainer added a commit that referenced this pull request Jul 25, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
jonathanrainer added a commit that referenced this pull request Jul 25, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
jonathanrainer added a commit that referenced this pull request Jul 26, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
jonathanrainer added a commit that referenced this pull request Jul 30, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
jonathanrainer added a commit that referenced this pull request Jul 31, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
aaronArinder pushed a commit that referenced this pull request Aug 1, 2024
Rather than fetching each subgraph one at a time,
make a single GraphQL request to get them all at
once.

This combines the work from #1985
@aaronArinder
Copy link
Contributor

added via #2001, closing

aaronArinder pushed a commit that referenced this pull request Aug 1, 2024
**NOTE: This should not be merged until #1874 has been merged, and then
this rebased on top**

In #1984 we added support to pass a `--graph-ref` flag to `rover dev`.
Since `supergraph compose` is performing the same function as `rover
dev` it makes sense to expose the same option there too. However, this
requires some refactoring in order to make work.

In principle the idea is fairly simple, we should centralise the
construction of the SupergraphConfig object so that both `dev` and
`supergraph compose` can hook in to it from a central location. However
to do this I've also had to add a fair amount of testing because there
was very little coverage in this area, however, due to @dotdat's work on
#1977 we have a lot more test coverage than before. That work has been
integrated into this PR.

Further more @dotdat was working on another PR #1985 , which optimises
the calls made by the `--graph-ref` option to fetch subgraphs in one
request rather than multiple. Due to being in this area and working on
the `--graph-ref` option it also made sense to integrate that PR here as
well, to prevent a complicated rebase later on.

This PR has been tested by the addition of unit tests and running
`supergraph compose` locally.

_There is also a small change to `deny.toml` in this PR to add support
for BSD-2-Clause licences. We already support BSD-3-Clause, and 2-Clause
is strictly a subset so this should not be a problem_
@jonathanrainer jonathanrainer removed this from the v0.26.0 milestone Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants