-
Notifications
You must be signed in to change notification settings - Fork 206
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
feat(http): GraphQL over HTTP compliance #1987
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1987 +/- ##
==========================================
- Coverage 82.28% 82.17% -0.12%
==========================================
Files 174 174
Lines 17809 17765 -44
==========================================
- Hits 14655 14599 -56
- Misses 3154 3166 +12 ☔ View full report in Codecov by Sentry. |
|
Report | Wed, May 22, 2024 at 11:35:03 UTC |
Project | tailcall |
Branch | 1987/merge |
Testbed | benchmarking-runner |
Click to view all benchmark results
Benchmark | Latency | Latency Results nanoseconds (ns) | (Δ%) | Latency Upper Boundary nanoseconds (ns) | (%) |
---|---|---|---|
group_by | ✅ (view plot) | 547.15 (-8.33%) | 650.25 (84.15%) |
input/args.missing | ✅ (view plot) | 22.93 (-7.10%) | 27.20 (84.30%) |
input/args.nested.existing | ✅ (view plot) | 42.39 (-17.93%) | 65.89 (64.33%) |
input/args.nested.missing | ✅ (view plot) | 37.62 (-2.25%) | 40.65 (92.56%) |
input/args.root | ✅ (view plot) | 35.75 (-25.79%) | 62.47 (57.24%) |
input/headers.existing | ✅ (view plot) | 32.63 (+2.64%) | 33.52 (97.34%) |
input/headers.missing | ✅ (view plot) | 33.27 (+8.03%) | 33.41 (99.58%) |
input/value.missing | ✅ (view plot) | 23.39 (-0.65%) | 25.33 (92.37%) |
input/value.nested.existing | ✅ (view plot) | 41.58 (+0.26%) | 43.92 (94.68%) |
input/value.nested.missing | ✅ (view plot) | 37.57 (+3.19%) | 38.35 (97.96%) |
input/value.root | ✅ (view plot) | 35.63 (-7.33%) | 40.39 (88.21%) |
input/vars.existing | ✅ (view plot) | 7.13 (-12.42%) | 9.07 (78.58%) |
input/vars.missing | ✅ (view plot) | 10.65 (+23.19%) | 11.23 (94.86%) |
test_batched_body | ✅ (view plot) | 2,696.50 (-99.67%) | 2,618,055.51 (0.10%) |
test_batched_body #2 | ✅ (view plot) | 1,746,500.00 (+0.64%) | 1,882,262.34 (92.79%) |
test_data_loader | ✅ (view plot) | 474,150.00 (+0.43%) | 490,187.70 (96.73%) |
test_handle_request | ✅ (view plot) | 184,360.00 (+9.02%) | 185,608.52 (99.33%) |
test_http_execute_method | ✅ (view plot) | 18,434.00 (+1.54%) | 19,024.57 (96.90%) |
with_mustache_expressions | ✅ (view plot) | 1,153.70 (-1.44%) | 1,234.88 (93.43%) |
with_mustache_literal | ✅ (view plot) | 683.49 (-5.51%) | 771.78 (88.56%) |
Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help
Action required: PR inactive for 2 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@morgery thanks for the PR. Few things —
- Compare the flamecharts wrt main and see what's causing the performance degradation.
- There is one pending warning —
86EE SHOULD use a status code of 400 on variable coercion failure when accepting application/graphql-response+json
Action required: PR inactive for 2 days. |
Action required: PR inactive for 2 days. |
Action required: PR inactive for 2 days. |
Moving to draft @mogery |
Action required: PR inactive for 2 days. |
PR closed after 5 days of inactivity. |
This is an issue deep in the GraphQL engine somewhere and I have no clue where to start debugging this but I'll try and look into it. Could you reopen the PR please? |
Summary:
Adds code to make Taicall GraphQL-over-HTTP compliant. Also converts the test suite over at GraphQL over HTTP into
execution_spec
tests.Issue Reference(s):
Fixes #1766
/claim #1766
Build & Testing:
cargo test
successfully../lint.sh --mode=fix
to fix all linting issues raised by./lint.sh --mode=check
.Checklist:
<type>(<optional scope>): <title>