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

Use required version of federation coreSpec rather than latest #2528

Merged
merged 8 commits into from
Jun 5, 2023

Conversation

clenfest
Copy link
Contributor

Important points

  • We get the the version of any core feature implied by that version of federation, unless there has been a major version bump
  • If there has been a major version bump of the core spec, we get the minimum required version that matches the major version of latest

Important points
- We get the the version of any core feature implied by that version of federation, unless there has been a major version bump
- If there has been a major version bump of the core spec, we get the minimum required version that matches the major version of latest
@changeset-bot
Copy link

changeset-bot bot commented Apr 14, 2023

🦋 Changeset detected

Latest commit: cfdd354

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@apollo/composition Minor
@apollo/federation-internals Minor
@apollo/gateway Minor
@apollo/query-planner Minor
@apollo/query-graphs Minor
@apollo/subgraph Minor
apollo-federation-integration-testsuite Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Apr 14, 2023

Deploy Preview for apollo-federation-docs canceled.

Name Link
🔨 Latest commit 0359c9a
🔍 Latest deploy log https://app.netlify.com/sites/apollo-federation-docs/deploys/646e25d787cf64000884038d

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 14, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@clenfest
Copy link
Contributor Author

Note to reviewers: I'm slightly unsure on two points.

  1. federation.ts still uses latest() for the tag/link/federation specs. I think this is ok, in almost all cases, since as far as I can tell Merger doesn't use a FederationBlueprint for the schema. However,
  2. Will this cause a problem with extractSubgraphFromSupergraph? I think it may, but I may have to give this some more thought.

@clenfest clenfest marked this pull request as ready for review April 14, 2023 03:51
@clenfest clenfest requested a review from a team as a code owner April 14, 2023 03:51
@clenfest clenfest changed the title First crack at an implementation of #2495 Use required version of federation coreSpec rather than latest Apr 14, 2023
Copy link
Member

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few minor suggestions.

Can we add some integration-ish tests for the composeServices fn which show various versions being used and the resulting supergraph?

Needs a changeset as well.

internals-js/src/coreSpec.ts Outdated Show resolved Hide resolved
internals-js/src/coreSpec.ts Outdated Show resolved Hide resolved
internals-js/src/coreSpec.ts Outdated Show resolved Hide resolved
internals-js/src/utils.ts Show resolved Hide resolved
@clenfest
Copy link
Contributor Author

So the reason I didn't put any tests into composeServices is that there isn't really anything to test just yet. Since we are saying that this isn't relevant for older versions, We'll always get the latest versions of joinSpec, etc. for now, and it won't become relevant until there is a new version of one of them.

@clenfest clenfest changed the base branch from main to next June 4, 2023 19:40
@clenfest clenfest merged commit 6b18af5 into next Jun 5, 2023
@clenfest clenfest deleted the clenfest/supergraph_versioning branch June 5, 2023 13:10
@github-actions github-actions bot mentioned this pull request Jun 5, 2023
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.

2 participants