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

fix: allow default values for input arguments #1939

Closed
wants to merge 7 commits into from
Closed

fix: allow default values for input arguments #1939

wants to merge 7 commits into from

Conversation

ssddOnTop
Copy link
Member

Issue Reference(s):
Fixes #1931
/claim 1931

@algora-pbc algora-pbc bot mentioned this pull request May 13, 2024
Open
2 tasks
@github-actions github-actions bot added the type: fix Iterations on existing features or infrastructure. label May 13, 2024
@ssddOnTop ssddOnTop marked this pull request as ready for review May 14, 2024 08:10
@codecov Codecov
Copy link

codecov bot commented May 14, 2024 •
edited

Codecov Report

Attention: Patch coverage is 93.10345% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 82.10%. Comparing base (630254a) to head (c6aaf67).

❗ Current head c6aaf67 differs from pull request most recent head 73b2720

Please upload reports for the commit 73b2720 to get more accurate results.

Files Patch % Lines
src/core/blueprint/into_schema.rs 93.10% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1939      +/-   ##
==========================================
- Coverage   82.79%   82.10%   -0.70%     
==========================================
  Files         183      173      -10     
  Lines       18243    17644     -599     
==========================================
- Hits        15104    14486     -618     
- Misses       3139     3158      +19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tusharmath tusharmath added the ci: benchmark Runs benchmarks label May 14, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 14, 2024 •
edited

🐰Bencher

ReportSat, May 18, 2024 at 03:45:12 UTC
Projecttailcall
Branch1939/merge
Testbedbenchmarking-runner

🚨 1 ALERT: Threshold Boundary Limit exceeded!
BenchmarkMeasure (units)ViewValueLower BoundaryUpper Boundary
input/vars.missingLatency (nanoseconds (ns))🚨 (view plot | view alert)11.73 (+37.26%)10.95 (107.14%)

Click to view all benchmark results
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)		Latency Upper Boundary
nanoseconds (ns) | (%)
group_by✅ (view plot)535.10 (-10.68%)648.29 (82.54%)
input/args.missing✅ (view plot)22.93 (-7.37%)27.22 (84.23%)
input/args.nested.existing✅ (view plot)42.66 (-18.05%)65.96 (64.67%)
input/args.nested.missing✅ (view plot)37.38 (-3.00%)40.63 (92.01%)
input/args.root✅ (view plot)37.92 (-21.97%)62.51 (60.67%)
input/headers.existing✅ (view plot)33.03 (+3.91%)33.50 (98.61%)
input/headers.missing✅ (view plot)31.55 (+2.47%)33.37 (94.52%)
input/value.missing✅ (view plot)23.19 (-1.54%)25.36 (91.41%)
input/value.nested.existing✅ (view plot)42.07 (+1.40%)43.95 (95.72%)
input/value.nested.missing✅ (view plot)36.84 (+1.19%)38.34 (96.09%)
input/value.root✅ (view plot)37.57 (-2.40%)40.38 (93.04%)
input/vars.existing✅ (view plot)7.44 (-8.94%)9.06 (82.05%)
input/vars.missing🚨 (view plot | view alert)11.73 (+37.26%)10.95 (107.14%)
test_batched_body✅ (view plot)2,682.70 (-99.69%)2,654,750.22 (0.10%)
test_batched_body #2✅ (view plot)1,634,100.00 (-6.06%)1,888,131.04 (86.55%)
test_data_loader✅ (view plot)460,710.00 (-2.46%)490,377.11 (93.95%)
test_handle_request✅ (view plot)157,400.00 (-7.82%)182,978.09 (86.02%)
with_mustache_expressions✅ (view plot)1,183.80 (+1.04%)1,236.24 (95.76%)
with_mustache_literal✅ (view plot)741.64 (+2.43%)772.57 (96.00%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

tusharmath
tusharmath reviewed May 15, 2024
View reviewed changes
src/core/blueprint/into_schema.rs Outdated
} else {
async_graphql_value::to_value(args).unwrap_or_default()
};

let mut dyn_schema_field = dynamic::Field::new(
Copy link
Contributor

Choose a reason for hiding this comment

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

Just call the the dyn_schema_field.arguments(args) Current implementation is a bit complicated and unnecessary.

@github-actions GitHub Actions
Copy link

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

@github-actions github-actions bot added state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. and removed state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. labels May 17, 2024
@ssddOnTop ssddOnTop marked this pull request as draft May 18, 2024 03:30
@tusharmath tusharmath removed the ci: benchmark Runs benchmarks label May 18, 2024
@github-actions GitHub Actions
Copy link

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

@github-actions github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label May 20, 2024
@ssddOnTop ssddOnTop removed the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label May 24, 2024
@ssddOnTop
Copy link
Member Author

there is an issue with async-graphql as well, I raised a PR async-graphql/async-graphql#1527 there if it gets merged, I'll finish touchups in this PR.

@github-actions GitHub Actions
Copy link

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

@github-actions github-actions bot added state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. and removed state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. labels May 27, 2024
@github-actions GitHub Actions
Copy link

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

@github-actions github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label May 29, 2024
@ssddOnTop ssddOnTop removed the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label May 30, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 1, 2024

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

@github-actions github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Jun 1, 2024
@ssddOnTop ssddOnTop closed this Jun 3, 2024
@ssddOnTop ssddOnTop deleted the chore/fix-def-as-args branch June 3, 2024 06:21
@shashitnak shashitnak mentioned this pull request Jun 6, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tusharmath tusharmath tusharmath left review comments

Assignees
No one assigned
Labels
🙋 Bounty claim state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. type: fix Iterations on existing features or infrastructure.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: allow default values for input arguments.
2 participants
@ssddOnTop @tusharmath