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

Introduce a router_service #2170

Merged
merged 107 commits into from
Dec 16, 2022
Merged

Introduce a router_service #2170

merged 107 commits into from
Dec 16, 2022

Conversation

o0Ignition0o
Copy link
Contributor

@o0Ignition0o o0Ignition0o commented Nov 28, 2022

Fixes #1496 eventually.

todo:

  • fix tests
  • enable APQ
  • deal with sandbox and homepage
  • decompression
  • tracing
  • impl TryFrom<supergraph::Request> for Request should put the graphql::Request in the body if the method is POST, and urlencode it in the path if the method is GET
  • sure could use a helper to go from a defer chunked payload into a stream of graphql::Response for test purposes (like take the bytes, strip --graphql and everything, and deserialize the json contents), possibly using https://github.com/scottlamb/multipart-stream-rs/blob/master/examples/client.rs#L35
  • move back the supergraph_service defer tests
  • not happy with from_supergraph_mock_callback_and_configuration and the likes, let's maybe use a builder
  • router::Request and router::Response builders
  • Update and verify Apollo telemetry

@github-actions

This comment has been minimized.

Copy link
Contributor

@garypen garypen left a comment

Choose a reason for hiding this comment

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

Huge amount of work. Really well done!!

Did you check the diffs of the documentation for public changes? Could you please document the result in the PR so we can see what has been added?

@BrynCooke
Copy link
Contributor

BrynCooke commented Dec 16, 2022

Here's a list of API changes: https://gist.github.com/BrynCooke/590941cc0008dabd40703aca3e9b4e6b

@o0Ignition0o
Copy link
Contributor Author

@BrynCooke thanks a lot for the diff

Removed items from the public API
=================================
(none)

Changed items in the public API
===============================
(none)

this looks good doesn't it? do we see things that went public and shouldnt ?

@garypen garypen self-requested a review December 16, 2022 11:41
@o0Ignition0o o0Ignition0o enabled auto-merge (squash) December 16, 2022 11:45
@o0Ignition0o o0Ignition0o merged commit 23be92d into dev Dec 16, 2022
@o0Ignition0o o0Ignition0o deleted the router_service branch December 16, 2022 12:16
@abernix abernix added this to the v1.7.0 milestone Dec 22, 2022
@abernix abernix mentioned this pull request Dec 23, 2022
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.

Pre-Supergraph Router service layer (HTTP)
7 participants