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

Disable async function as group and check callback #2875

Merged
merged 5 commits into from
Jan 31, 2023

Conversation

mstoykov
Copy link
Contributor

The general inconsistencies between group as an idea and asynchronous code makes it not great fit and such we will try to discourage users to mix them.

Updates #2728

@mstoykov mstoykov added this to the v0.43.0 milestone Jan 25, 2023
@github-actions github-actions bot requested review from codebien and imiric January 25, 2023 15:14
@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2023

Codecov Report

Merging #2875 (533cdcc) into master (1d99b0b) will decrease coverage by 0.15%.
The diff coverage is 81.92%.

❗ Current head 533cdcc differs from pull request most recent head 3142bad. Consider uploading reports for the commit 3142bad to get more accurate results

@@            Coverage Diff             @@
##           master    #2875      +/-   ##
==========================================
- Coverage   76.88%   76.74%   -0.15%     
==========================================
  Files         219      225       +6     
  Lines       16666    16811     +145     
==========================================
+ Hits        12814    12901      +87     
- Misses       3044     3083      +39     
- Partials      808      827      +19     
Flag Coverage Δ
ubuntu 76.66% <81.92%> (-0.02%) ⬇️
windows 76.47% <81.92%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
api/v1/setup_teardown_routes.go 58.82% <60.00%> (+0.92%) ⬆️
js/modules/k6/experimental/tracing/client.go 61.11% <61.11%> (ø)
js/modules/k6/experimental/tracing/options.go 63.63% <63.63%> (ø)
api/v1/status_routes.go 34.69% <75.00%> (-19.48%) ⬇️
js/modules/k6/experimental/tracing/module.go 75.55% <75.55%> (ø)
js/modules/k6/experimental/tracing/trace_id.go 78.94% <78.94%> (ø)
api/v1/routes.go 55.31% <90.00%> (ø)
core/engine.go 85.00% <92.85%> (-9.34%) ⬇️
metrics/engine/engine.go 84.82% <94.87%> (+4.05%) ⬆️
execution/scheduler.go 92.83% <96.15%> (+0.55%) ⬆️
... and 24 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

The general inconsistencies between group as an idea and
asynchronous code makes it not great fit and such we will try to
discourage users to mix them.

The same is done to check for consistency and as otherwise an async
function will always give back true as it returns a promise which is
cast to boolean.

Updates #2728
@mstoykov mstoykov force-pushed the asyncGroupForbidden branch from fb87f90 to 5b483aa Compare January 26, 2023 14:31
@mstoykov mstoykov changed the title Disable async function as group callback Disable async function as group and check callback Jan 27, 2023
@mstoykov mstoykov requested a review from na-- January 27, 2023 10:07
js/modules/k6/k6.go Outdated Show resolved Hide resolved
Co-authored-by: na-- <n@andreev.sh>
js/modules/k6/k6.go Outdated Show resolved Hide resolved
@mstoykov mstoykov requested review from na-- and andrewslotin January 31, 2023 10:05
js/modules/k6/k6.go Outdated Show resolved Hide resolved
na--
na-- previously approved these changes Jan 31, 2023
Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

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

LGTM except the minor error message nitpick, to make this error match the other ones.

codebien
codebien previously approved these changes Jan 31, 2023
Co-authored-by: na-- <n@andreev.sh>
@mstoykov mstoykov dismissed stale reviews from codebien and na-- via f3b7823 January 31, 2023 12:37
na--
na-- previously approved these changes Jan 31, 2023
codebien
codebien previously approved these changes Jan 31, 2023
@mstoykov mstoykov dismissed stale reviews from codebien and na-- via 3142bad January 31, 2023 13:03
@mstoykov mstoykov requested review from codebien and na-- January 31, 2023 13:03
@mstoykov mstoykov merged commit 8da4149 into master Jan 31, 2023
@mstoykov mstoykov deleted the asyncGroupForbidden branch January 31, 2023 13:21
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.

5 participants