-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Update sobek to fix interrupt with async code #4017
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also fix it through the codebase, although experimental/streams and fs likely will need more work.
mstoykov
requested review from
oleiade and
olegbespalov
and removed request for
a team
October 25, 2024 07:38
mstoykov
added a commit
to grafana/xk6-websockets
that referenced
this pull request
Oct 25, 2024
5 tasks
mstoykov
added a commit
to grafana/xk6-redis
that referenced
this pull request
Oct 25, 2024
This "automatically" fixes grafana/k6#4017 when this is build with a k6 version after that PR is merged. Also reduces copy-pasted code.
mstoykov
added a commit
to grafana/xk6-browser
that referenced
this pull request
Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and Interrupt will work. AbortingPromises seems to have not been used at all and just adds complexity. And `promises.New()` already does the remaining things needed to happen, so this can be merged even without grafana/k6#4017.
mstoykov
added a commit
to grafana/xk6-browser
that referenced
this pull request
Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and Interrupt will work. AbortingPromises seems to have not been used at all and just adds complexity. And `promises.New()` already does the remaining things needed to happen, so this can be merged even without grafana/k6#4017.
3 tasks
mstoykov
added a commit
to grafana/xk6-browser
that referenced
this pull request
Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and Interrupt will work. AbortingPromises seems to have not been used at all and just adds complexity. And `promises.New()` already does the remaining things needed to happen, so this can be merged even without grafana/k6#4017.
olegbespalov
approved these changes
Oct 25, 2024
oleiade
approved these changes
Oct 25, 2024
mstoykov
added a commit
to grafana/xk6-websockets
that referenced
this pull request
Oct 25, 2024
ankur22
pushed a commit
to grafana/xk6-browser
that referenced
this pull request
Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and Interrupt will work. AbortingPromises seems to have not been used at all and just adds complexity. And `promises.New()` already does the remaining things needed to happen, so this can be merged even without grafana/k6#4017.
ankur22
pushed a commit
to grafana/xk6-browser
that referenced
this pull request
Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and Interrupt will work. AbortingPromises seems to have not been used at all and just adds complexity. And `promises.New()` already does the remaining things needed to happen, so this can be merged even without grafana/k6#4017.
mstoykov
added a commit
to grafana/xk6-websockets
that referenced
this pull request
Oct 25, 2024
mstoykov
added a commit
to grafana/xk6-redis
that referenced
this pull request
Oct 26, 2024
This "automatically" fixes grafana/k6#4017 when this is build with a k6 version after that PR is merged. Also reduces copy-pasted code.
Merged
oleiade
added a commit
that referenced
this pull request
Nov 11, 2024
* Initialize v0.55.0 release notes * Update release notes for browser 1448 * Update release notes for browser page.on generalization * Update release notes for browser 1439 * Update release notes for browser 1461 * Update release notes for browser 1496 I'm not yet sure if we should add this PR as it's a part of a bigger functionality that we expect to have in k6 v0.56. See the issue for more details: grafana/xk6-browser#1289 * Replace issue 1461 with PR 1462 * release notes: webcrypto RSA support * release notes: statsd output removal * release notes: outputs changes * Add browser related changes to release notes * Remove template examples from release notes * Add browser#1457 to release notes * release notes: updates check recommendation * Add initial page.on('metric') release note change * Add issues to the page.on('metric') release note * release notes: add TLA support section * release notes: add tracing breaking change and future open ones * auto-assigner changelog * check recommendation apply feedback * fixup! release notes: add tracing breaking change and future open ones * Apply release notes clean-ups and fillers * Address wrong formulation of tracing removal * Update release notes/v0.55.0.md Co-authored-by: Oleg Bespalov <oleg.bespalov@grafana.com> * Apply suggestions from code review Co-authored-by: Ankur <ankur.agarwal@grafana.com> * Apply suggestions from code review Co-authored-by: Ankur <ankur.agarwal@grafana.com> * Apply pull request review suggestions * Update the page.on('metric') description * Update release notes/v0.55.0.md Co-authored-by: Ankur <ankur.agarwal@grafana.com> * Update page.on example as an expandable detail * Align last sections format with template * Update ControlOrMeta description * Update wording of page.on example * Apply pull request review suggestions * Document #4017 bug fix * Update release notes/v0.55.0.md Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com> * Bring minor improvement to #4009 --------- Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com> Co-authored-by: Oleg Bespalov <oleg.bespalov@grafana.com> Co-authored-by: ankur22 <ankur.agarwal@grafana.com> Co-authored-by: Mihail Stoykov <M.Stoikov@gmail.com> Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Also fix it through the codebase, although experimental/streams and fs likely will need more work.
As part of this change
NewPromise
's resolve and reject function now return error and that error is going to handle the interrupt - but probably it should be handled as any other error.This mostly needs that it needs to be propagated upwards.
This also needs to be changed in different extensions, although there the work seems to be less.
Why?
Fix #3983 among other things.
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)
Fixes #3983