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

async_hooks: don't read resource if ALS is disabled #34617

Closed

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Aug 3, 2020

Only call executionAsyncResource() in getStore() if the ALS instance is enabled because the resource is not needed otherwise.

Only call executionAsyncResource() in getStore() if the ALS instance is
enabled because the resource is not needed otherwise.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the async_hooks Issues and PRs related to the async hooks subsystem. label Aug 3, 2020
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32624/

@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 4, 2020
@Flarna Flarna changed the title [async_hooks] don't read resource if ALS is disabled async_hooks: don't read resource if ALS is disabled Aug 4, 2020
Flarna added a commit that referenced this pull request Aug 6, 2020
Only call executionAsyncResource() in getStore() if the ALS instance is
enabled because the resource is not needed otherwise.

PR-URL: #34617
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Flarna
Copy link
Member Author

Flarna commented Aug 6, 2020

Landed in a4e1755

@Flarna Flarna closed this Aug 6, 2020
@Flarna Flarna deleted the als_rm_unneeded_call branch August 6, 2020 21:31
addaleax pushed a commit that referenced this pull request Aug 8, 2020
Only call executionAsyncResource() in getStore() if the ALS instance is
enabled because the resource is not needed otherwise.

PR-URL: #34617
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@codebytere codebytere mentioned this pull request Aug 10, 2020
codebytere pushed a commit that referenced this pull request Aug 11, 2020
Only call executionAsyncResource() in getStore() if the ALS instance is
enabled because the resource is not needed otherwise.

PR-URL: #34617
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Only call executionAsyncResource() in getStore() if the ALS instance is
enabled because the resource is not needed otherwise.

PR-URL: #34617
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Only call executionAsyncResource() in getStore() if the ALS instance is
enabled because the resource is not needed otherwise.

PR-URL: #34617
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants