Skip to content

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Oct 29, 2024

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Updated @humanwhocodes/retry and set concurrency to 100.

fixes #18977

Is there anything you'd like reviewers to focus on?

I didn't add any new tests for this because I couldn't come up with one that failed without it. Functionally, this should result in no difference to the end user and assuming our existing tests pass, I think we're okay.

@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Oct 29, 2024
@github-actions github-actions bot added cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features labels Oct 29, 2024
Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 4974630
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/6724ea8cae703800086c5469

@nzakas nzakas mentioned this pull request Oct 29, 2024
1 task
@nzakas nzakas marked this pull request as ready for review October 31, 2024 21:17
@nzakas nzakas requested a review from a team as a code owner October 31, 2024 21:17
const controller = new AbortController();
const retryCodes = new Set(["ENFILE", "EMFILE"]);
const retrier = new Retrier(error => retryCodes.has(error.code));
const retrier = new Retrier(error => retryCodes.has(error.code), { signal: controller.signal, concurrency: 100 });
Copy link
Member Author

Choose a reason for hiding this comment

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

I set concurrency to 100 to match @fasttime's prototype. I tried with values from 100-1000 and found performance was basically the same. We can always tweak this later if we the need.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

If possible, would like @fasttime to verify before today's release.

@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Nov 1, 2024
Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

LGMT, thanks!

@fasttime fasttime merged commit 24d0172 into main Nov 1, 2024
26 checks passed
@fasttime fasttime deleted the issue18977 branch November 1, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Bug: EMFILE: too many open files using Flat Config with very large projects

4 participants