feat(gateway): Update managed federation to fetch "cloud config" #458
Conversation
|
FYI the stop stuff is on main (and is released as v0.23.0) — hope that helps! |
|
Should I wait until that is done before I review? Also is this based on the newest version of the join/core spec, or is this just about changing how we talk to managed federation? |
abernix
changed the title
This is mostly a fairly large update / revamp to the networkRequests test file. Previously skipped /flaky tests are now up-to-date and functioning as expected! Some smaller changes were made to the gateway: i.e. the stop() function was updated to actually do something. It doesn't stop the gateway process or even from serving requests, but it does stop it from polling. This behavior was needed for the sake of testing so that gateways still running in tests didn't overpoll and ruin network mocks. ^additional commentary on above: this was only my suspicion and my changes seem to have resolved the issue. Though I don't fully understand why this wasn't a problem in the first place - or maybe it was and was causing the flakiness in the past? I don't know.
trevor-scheer
force-pushed
the
trevor/fetch-csdl
branch
from
ef33764
to
7d11ace
Compare
|
@glasser the Nothing to do with core / join yet. |
| AUTHENTICATION_FAILED = 'AUTHENTICATION_FAILED', | ||
| ACCESS_DENIED = 'ACCESS_DENIED', | ||
| UNKNOWN_REF = 'UNKNOWN_REF', | ||
| RETRY_LATER = 'RETRY_LATER', |
There was a problem hiding this comment.
Should we handle this retry later specially?
There was a problem hiding this comment.
Yes, though it sounds like Gravity has some changes in the works and I'm unsure what these errors are going to look like. I'm going to hold off for now and see if this concept even still exists in the next iteration.
* Don't log and rethrow an error which is already handled further up * Include original error message and originating service in health check error message
trevor-scheer
force-pushed
the
trevor/fetch-csdl
branch
from
ac55b45
to
692e046
Compare
There was a problem hiding this comment.
Yeah this all basically looks good, with the only serious concerns being the env var vs option precedence and not wanting to use graphql-tag's unbounded cache (plus the csdl naming which you were planning to get to lately). Pretty sure clicking that green button will be easy on Monday, but I'm out for the weekend. Have a nice weekend!
trevor-scheer
force-pushed
the
trevor/fetch-csdl
branch
from
9eb40e2
to
9d4f358
Compare
Yes, I've requested it be non-nullable :) |
There was a problem hiding this comment.
ok:
- typing fix you're working on already
- quick test suggestion
- comment deletion
- request for comment-and-link-to-issue for extraneous parse call
- concern about quality of errors which could be comment-and-link-to-issue as well
I'd click the green button now but we're both around and can get a last round of back-and-forth in quickly.
This PR moves the gateway over to a simplified startup and updating mechanism in managed mode. With the gateway now able to consume CSDL as input, it's no longer necessary for the gateway to perform a series of fetches in order to collect each service's partial SDL and do composition on the fly.
In order to take full advantage of this capability, we will serve CSDL straight to the gateway rather than all of the individual partials.
This feature is being implemented as opt-in only for now, and should not be used by the general public. The means to opt-in is by providing appropriate configuration by either defining:
Via gateway configuration:
experimental_schemaConfigDeliveryEndpoint: stringORVia env var:
APOLLO_SCHEMA_CONFIG_DELIVERY_ENDPOINTRelated prior work in preparation for this PR:
#278
#440
Note to the reviewer: this PR currently includes unrelated work (gatewaystopfunctionality) which is already addressed in #452. Anystoprelated code in this PR can effectively be ignored since it'll be replaced by @glasser 's implementation once it lands.Resolved.
TODO:
stopbehavior (simplifies the scope of this PR)Finalize / productionize our new CSDL delivery mechanismFinal TODO is no longer a requirement since new delivery mechanism is opt-in, meaning we can ship this with no effect for our users for now.