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

Add cost estimation wrapper #1180

Merged
merged 8 commits into from
Dec 2, 2021

Conversation

paulpdaniels
Copy link
Collaborator

Adds a simple cost estimation wrapper for computing the effective cost of executing a query and rejecting it if it is too expensive. Different from maxDepth or maxFields because it supports an arbitrary function to determine the cost of the field. Out of the box it provides a simple cost directive that can be added to fields or types to specify their overall cost to execute. Note, because it is a "speculative" cost it can't take advantage of the actual return values meaning that it relies on the user to apply the correct multipliers and to determine a good metric for assessing a fields cost.

Copy link
Collaborator

@frekw frekw left a comment

Choose a reason for hiding this comment

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

I think it looks really really nice. Both the directives and the multipliers are much nicer than other solutions I've seen. Great work @paulpdaniels! 👏

@ghostdogpr
Copy link
Owner

(will review tomorrow or thursday, when work gets quieter 😄)

Copy link
Owner

@ghostdogpr ghostdogpr left a comment

Choose a reason for hiding this comment

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

This is great 👍

I have some minor comments. What about the remaining TODOs?

vuepress/docs/docs/middleware.md Outdated Show resolved Hide resolved
vuepress/docs/docs/middleware.md Show resolved Hide resolved
core/src/main/scala/caliban/wrappers/CostEstimation.scala Outdated Show resolved Hide resolved
Copy link
Owner

@ghostdogpr ghostdogpr left a comment

Choose a reason for hiding this comment

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

Awesome!

@paulpdaniels paulpdaniels merged commit 771aa9a into ghostdogpr:master Dec 2, 2021
@paulpdaniels paulpdaniels deleted the cost-estimation branch December 2, 2021 11:37
Fluxx pushed a commit to Fluxx/caliban that referenced this pull request Jan 24, 2022
Fluxx pushed a commit to Fluxx/caliban that referenced this pull request Jan 27, 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.

3 participants