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.

Sign up for GitHub

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

Jump to bottom

remove Buffer from query deduplication #1889

Merged
merged 4 commits into from
Sep 26, 2022
Merged

remove Buffer from query deduplication #1889

merged 4 commits into from
Sep 26, 2022

Conversation

Geal
Copy link
Contributor

@Geal Geal commented Sep 26, 2022

This removes another use of Buffer in the hot path

Geoffroy Couprie added 2 commits September 26, 2022 14:43
@Geal Geal requested a review from garypen September 26, 2022 12:44
garypen
garypen reviewed Sep 26, 2022
View reviewed changes
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.

Does that mean nothing is using ServiceBuilderExt::buffered() now? We could maybe mark it as deprecated if so.

apollo-router/src/router_factory.rs Outdated Show resolved Hide resolved
@Geal
Copy link
Contributor Author

Geal commented Sep 26, 2022

ServiceBuilderExt::buffered() is still useful when plugin authors want to use checkpoint_async so I think we should leave it. But now with this PR for stock router usage there's no more Buffer

@garypen garypen requested review from garypen and bnjjj September 26, 2022 12:50
@Geal Geal enabled auto-merge (squash) September 26, 2022 13:30
@Geal Geal merged commit 2836117 into dev Sep 26, 2022
@Geal Geal deleted the geal/remove-buffer-in-dedup branch September 26, 2022 13:34
@abernix abernix added this to the v1-NEXT milestone Sep 28, 2022
This was referenced Sep 29, 2022
Merged
Merged
@Geal Geal mentioned this pull request Oct 26, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@o0Ignition0o o0Ignition0o o0Ignition0o approved these changes

@bnjjj bnjjj bnjjj approved these changes

@garypen garypen garypen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Geal @garypen @bnjjj @o0Ignition0o @abernix