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

Set supportedFeatures appropriately when calling Supergraph.build(). #403

Merged
merged 4 commits into from
Aug 30, 2023

Conversation

sachindshinde
Copy link
Contributor

Router supports the authenticated v0.1 spec and the requiresScopes v0.1 spec, but these aren't included in the default specs used by Supergraph.build() (i.e. DEFAULT_SUPPORTED_SUPERGRAPH_FEATURES), so usages of these specs cause errors.

This PR changes the supportedFeatures passed to Supergraph.build() to include 'https://specs.apollo.dev/authenticated/v0.1' and 'https://specs.apollo.dev/requiresScopes/v0.1' in addition to the default specs.

@sachindshinde
Copy link
Contributor Author

@trevor-scheer @clenfest For some reason, we're getting this error while compiling v8.

info: (cargo) | error[E0080]: evaluation of constant value failed
info: (cargo) |     --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/v8-0.60.1/src/isolate.rs:1724:3
info: (cargo) |      |
info: (cargo) | 1724 |   assert!(size_of::<TypeId>() == size_of::<u64>());
info: (cargo) |      |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: size_of::<TypeId>() == size_of::<u64>()', /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/v8-0.60.1/src/isolate.rs:1724:3
info: (cargo) |      |
info: (cargo) |      = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)
info: (cargo) | 
info: (cargo) | For more information about this error, try `rustc --explain E0080`.
info: (cargo) | error: could not compile `v8` (lib) due to previous error
info: (cargo) | warning: build failed, waiting for other jobs to finish...
info: (cargo) | error: could not compile `v8` (lib) due to previous error
Error: 'cargo clippy -- -D warnings' failed with exit status: 101.

Exited with code exit status 1

Have any of y'all seen this before?

@Geal
Copy link
Contributor

Geal commented Aug 30, 2023

this is likely the same issue apollographql/router#3684

@Geal Geal mentioned this pull request Aug 30, 2023
@Geal
Copy link
Contributor

Geal commented Aug 30, 2023

@sachindshinde the v8 build error is fixed now with #404

REQUIRES_SCOPES_VERSIONS,
} from "@apollo/federation-internals";

const authenticatedV01Feature = AUTHENTICATED_VERSIONS.find(
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in fedutil-libs. I think we'd always just want the latest, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@clenfest Presumably if we have multiple versions for a given identity (e.g. v0.1 and v0.2), we'd want all of them instead of the latest. (Or at least, we'd want Supergraph.build() to be able to parse older versions to enable in-place upgrades.) There's not a public accessor for FeaturesDefinitions._definitions unfortunately, but we have FeatureDefinitions.versions(), so I've updated the PR to use that.

@sachindshinde sachindshinde force-pushed the sachin/use-supported-features-supergraph-build branch from 0a8d470 to 6ef4f11 Compare August 30, 2023 17:56
@sachindshinde sachindshinde requested a review from clenfest August 30, 2023 18:08
@sachindshinde sachindshinde merged commit 8b1a03e into main Aug 30, 2023
@sachindshinde sachindshinde deleted the sachin/use-supported-features-supergraph-build branch August 30, 2023 18:37
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