Disable npm update-notifier during module-load version check#69769
Open
ggiesen wants to merge 1 commit into
Open
Disable npm update-notifier during module-load version check#69769ggiesen wants to merge 1 commit into
ggiesen wants to merge 1 commit into
Conversation
npm.py runs `npm --version` from _check_valid_version(), which is called by __virtual__() at module-load time whenever an npm binary is on PATH. npm's update-notifier tries to reach the registry, and behind a firewall or proxy that blocks it the command hangs for roughly three minutes, stalling every salt-call regardless of whether the npm module is used. Pass NO_UPDATE_NOTIFIER=1 in the command environment so the version check returns immediately. Fixes saltstack#59520
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 does this PR do?
salt/modules/npm.pyrunsnpm --versionfrom_check_valid_version(), which is invoked by__virtual__()at module-load time whenever annpmbinary is on PATH. npm's update-notifier tries to reach the registry on that call; behind a corporate firewall or proxy that blocks it, the command hangs for roughly three minutes before timing out, stalling every salt-call regardless of whether the npm module is actually used.This passes
env={"NO_UPDATE_NOTIFIER": "1"}to thatcmd.runcall so the version check returns immediately. The change is purely additive and does not alter the version-gating logic.What issues does this PR fix or reference?
Fixes #59520
Previous Behavior
Any command that loads execution modules (even ones unrelated to npm) blocked for about three minutes when an
npmbinary was present and the npm registry was unreachable, because_check_valid_version()rannpm --versionwith the update-notifier enabled.New Behavior
_check_valid_version()runsnpm --versionwithNO_UPDATE_NOTIFIER=1, so npm skips the registry check and the command returns immediately. Module load is no longer delayed behind a firewall/proxy.Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
changelog/59520.fixed.mdtests/pytests/unit/modules/test_npm.py:test_check_valid_version_disables_update_notifier_59520asserts the version check passesenv={"NO_UPDATE_NOTIFIER": "1"}(fails without the fix)test_check_valid_version_raises_on_old_npm_59520(inverse) confirms the version gate still raises on npm < 1.2test_check_valid_version_accepts_recent_npm_59520confirms a valid version returns without errorCommits signed with GPG?
No