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

Import JavaScript @apollo/federation and @apollo/gateway from the apollo-server repository. #134

Merged
merged 515 commits into from
Sep 10, 2020

Conversation

abernix
Copy link
Member

@abernix abernix commented Sep 7, 2020

With the renaming of this repository from rust to federation, this repository is slated to become the home for all Apollo Federation related concerns — including the current JavaScript implementations. Migrating these to a single repository will allow us to track conversations and development in a single place without being coupled to the Apollo Server repository.

This has become particularly important as Apollo Federation has grown into its own product over the course of the last year, which can evolve separate from Apollo Server. In practice, changes have rarely affected or needed to be coupled to Apollo Server directly, apart from the initial support for accepting a gateway parameter on the ApolloServer constructor. We also see the increasing need for Apollo Gateway and Federation progress to be tracked separately from Apollo Server, since they tend to attract different segments of the community than a standalone GraphQL server does.

Therefore, this PR brings the @apollo/gateway and @apollo/federation packages which currently live in the Apollo Server repository along with the relevant JavaScript infrastructure for building them (with TypeScript) and testing them (with Jest).

This is comprised of:

  • A git filter-branch of commits on the apollo-server repository

    (Though technically I used the git-filter-repo wrapper for it for the first time, with great success.)

    In particular, targeting commits which have touched the packages/apollo-gateway and packages/apollo-federation packages. This preserves the entire history of commits from that repository. To create links to those for posterity, I've amended the commits with Apollo-Orig-Commit-AS metadata and also replaced any relative issue/PR numbers in those commit messages (e.g., #1234) with absolute GitHub references pointing to the same (i.e., apollographql/apollo-server#1234). And I did the same for bare commit identifiers when possible! (e.g, abc123 👉 apollographql/apollo-server@abc123).

  • The CircleCI configuration to run on the apollographql/federation repo

    While we use GitHub Actions on the Federation repository today, the amount of automation for testing and publishing (including the generation of tarballs) which is already written into CircleCI configuration files is somewhat extensive right now and would require a fair bit of porting to make it exclusively GitHub Actions-based. (Perhaps a chore for another year.)

  • VSCode editor preferences for editor rules which must be enforced

    This is just for line-endings, tab-rules, newline-at-end-of-file things, since VS Code doesn't support .editorconfig (by default). I realize that most folks are using IntelliJ at the moment, so this is just for completeness to be on-par with the previous configuration. It parallels the .idea/ configuration that is already present, but no attempts to modify that configuration have been made.

  • Lerna configuration.

    This is for publishing and management of npm versions since all of the tooling for such should be automated right now via the above CircleCi bits.

Affected PRs

Moved to #141 where it will be tracked/evolved further.

Next steps

I can open any number of these as separate issues for tracking once we advance this PR.

See issue links next to ticked check box items for follow-up issues.

What to review?

Since this is largely a replay of commits from the apollo-server repository, nothing code-wise needs to be reviewed besides the last several commits on this branch.
— listed here for your convenience!:

abernix and others added 30 commits February 20, 2020 20:11
…ble.

This doesn't comprehensively fix places where we use `console` but `logger`
isn't already in scope, but these are easy wins.

cc @trevor-scheer

Apollo-Orig-Commit-AS: apollographql/apollo-server@397a735
…llographql/apollo-server#3829)

These types are compatible, but this change is necessary to account for the
fact that, while we have provided types for `make-fetch-happen` which matter
during development of Apollo Server, we don't include those types in the
distribution.  We could do that, of course, but as I'll note in a minute, we
want something more generic anyhow.

The omission of the types in the published package (currently, before this
commit) results in typing errors for consumers of `@apollo/gateway` since
the `import` statement for the `Fetcher` type from `make-fetch-happen` is
still emitted (see attached [[Screenshot]]).

Switching to the type that `apollo-serve-env` already provides via its
`fetch` implementation (literally, `typeof fetch`) should be a no-op change
and actually provide the more generic compatibility we want for those who
want to bring-their-own-fetch.  This should be the same as any type provided
by other Fetch-compatible packages as well, including `isomorphic-fetch`,
etc.

[Screenshot]: https://cc.jro.cc/E0uqEXrX
Apollo-Orig-Commit-AS: apollographql/apollo-server@daac9ee
* Centralize mocking fixtures and reuse them
* Simplify mock request signatures for more easily read and written tests
* Create nock/nockSuccess pairs for the most common use case and flexibility

cc @abernix

Apollo-Orig-Commit-AS: apollographql/apollo-server@4155e73
This message was being issued when a new server starts up, prior to ever having
a schema, when a storage secret (or any other artifact) can't be fetched
from GCS (or any error within `updateServiceDefinitions`) despite the fact
that there may be no `previousSchema`.

While I could use `previousSchema` to change the error message, I don't
think that it's this methods concern to decide what happens in the event of
an error.  I think it's only this methods concern to actually do the update,
if it is in fact successful.

Apollo-Orig-Commit-AS: apollographql/apollo-server@06c3267
Currently, we log the error and just return without throwing which causes
`load` (one of two places where `updateComposition` is called to not
actually fail and follow-up logic then suggests "Gateway successfully loaded
schema", even though that cannot be true.

The nice thing about this change (in addition to allowing someone to `catch`
an error from `ApolloGateway.prototype.load`) is that this should bubble all
the way up to the place where we currently call `load` within Apollo
Server's constructor, and stop the server from starting in this failure
condition:

https://github.com/apollographql/apollo-server/blob/7dcee80ff33061c0911458d593ebbca5a9c73939/packages/apollo-server-core/src/ApolloServer.ts#L366

The other place we call `updateComposition` is within a `setTimeout`.  That
particular case is less troublesome since it will retry on the next interval.

Apollo-Orig-Commit-AS: apollographql/apollo-server@1655f4b
No need to have them so far away from their usage and to even do these
assignments at all when there are already escape routes that exit this code
path.

Apollo-Orig-Commit-AS: apollographql/apollo-server@7250bfe
* Refer to the thing we are processing consistently as "schema definitions".
* Also make them generally more factual.

Apollo-Orig-Commit-AS: apollographql/apollo-server@c1ee91e
Rely on `message` only if it's present, and fall back to serialization
methods which might exist on the prototype otherwise (e.g. `toJSON`,
`toString`).

Also, switch to a single parameter usage of the logging facility.  While
`console.log` supports it, its certainly possible that the logger will
_not_, and will need that positional parameter for something else.

Apollo-Orig-Commit-AS: apollographql/apollo-server@e1d97d3
While successful `updateComposition` invocations were working properly,
failed invocations (including GCS access errors, network hiccups and just
general configuration mistakes) were currently cluttering the logs with
warnings of unhandled promise rejections.

While those unhandled rejections did include the actual error messages, this
adjusts them to be caught and logged in a structured manner with our
`logger`, sparing our logs from those unnecessarily verbose (and scary!)
messages.

Apollo-Orig-Commit-AS: apollographql/apollo-server@da06fa8
Overall, the success/failure behavior should be expected to be similar for
all of these requests, as they all access the same GCS store.

Apollo-Orig-Commit-AS: apollographql/apollo-server@ee4c9ac
…cts.

Because we want the actual executor to work when a schema might eventually
become available, as it may when `onSchemaChange` hooks eventually succeed.

Apollo-Orig-Commit-AS: apollographql/apollo-server@f878c7f
To correct the syntax error.

Co-Authored-By: Trevor Scheer <[email protected]>

Apollo-Orig-Commit-AS: apollographql/apollo-server@554e20c
@glasser
Copy link
Member

glasser commented Sep 10, 2020

Minor suggestion: Update the names of the GitHub action status checks to be clear that they only apply to the Rust code? It was a bit surprising to see things like Lint and Tests / Compile tests (pull_request) succeeding when I pushed a branch where I knew @apollo/gateway didn't build.

abernix added a commit to apollographql/apollo-server that referenced this pull request Sep 11, 2020
Everything will continue, however, as federation becomes more independent
from Apollo Server, it (and its concerns, like Apollo Gateway) move to their
own repository.  (It's always been relatively independent, but it didn't
necessarily warrant its own repository before.)

This includes `apollo-federation-integration-testsuite` (unpublished to
npm), `@apollo/gateway (which lived in `packages/apollo-gateway` here and as
`/gateway-js` on the new repository), and `@apollo/federation` (which lived
in `packages/apollo-federation` here and as `/federation-js` on the new
repository).

See #4560 for details,
and also apollographql/federation#134 for where (and
how) it was introduced on the other side.
@@ -23,7 +23,6 @@
"@apollo/query-planner-wasm": "0.0.2",
"@types/node-fetch": "2.5.4",
"apollo-engine-reporting-protobuf": "^0.5.2",
"apollo-env": "^0.6.1",
Copy link
Member

Choose a reason for hiding this comment

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

Love to see it!

abernix added a commit that referenced this pull request Sep 15, 2020
abernix added a commit that referenced this pull request Sep 15, 2020
This brings the existing documentation and its supporting infrastructure (in
this case, modeled after the Apollo Server repository, but this is standard
"Apollo infrastructure" in the general sense) to the Federation repository
where it can live alongside the code that was migrated over in
#134.

Related: #142
Related: #143
This was referenced Sep 16, 2020
abernix added a commit that referenced this pull request Sep 17, 2020
… repo. (#158)

This brings the existing documentation and its supporting infrastructure (in
this case, modeled after the Apollo Server repository, but this is standard
"Apollo infrastructure" in the general sense) to the Federation repository
where it can live alongside the code that was migrated over in
#134.

Related: #142
Related: #143
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.