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

npm state module: Add parameter to specify a custom NPM server #11926

Merged
merged 1 commit into from
Apr 15, 2014

Conversation

vortec
Copy link
Contributor

@vortec vortec commented Apr 14, 2014

The pip-state state module offers the parameters index_url and extra_index_url to pass a custom PyPI server to Salt.

The npm state module is lacking such an option, therefore I cannot use my companies NPM server to manage private NodeJS packages.

I think a parameter to specify a custom NPM server would greatly improve the npm state module.

@cachedout cachedout added this to the Approved for future release milestone Apr 11, 2014
@cachedout
Copy link
Contributor

Agreed. This is a great suggestion. I'm approving it for a future release.

@vortec
Copy link
Contributor Author

vortec commented Apr 14, 2014

I attached some code to this issue. Not sure if and when Travis picks this up, a manual review would be appreciated too. :)

@ghost
Copy link

ghost commented Apr 14, 2014

Test Failed.

If the failures are unrelated to your code, don't stress, a core developer will know these apart.
In the future, if possible, please branch off a passing commit to avoid false positives.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/3375/

@s0undt3ch
Copy link
Collaborator

Code-wise, I see no issues with the code in this PR, if functionality is there too(ie, you tested and it does it's job), then I give my 👍

Since @cachedout was the first developer commenting this, I think we should wait for his comments and final decision.

@s0undt3ch
Copy link
Collaborator

Actually, the registry keyword argument in the docstring should include .. versionadded:: XYZ

XYZ is the version, if we consider this a bugfix, then 2014.1.2 or 2014.1.3 if 2014.1.2 is released soon. Otherwise, Helium.

@cachedout
Copy link
Contributor

I think this is great. Let's merge it!

On Mon, Apr 14, 2014 at 3:44 PM, Pedro Algarvio notifications@github.comwrote:

Actually, the registry keyword argument in the docstring should include ..
versionadded:: XYZ

XYZ is the version, if we consider this a bugfix, then 2014.1.2 or
2014.1.3 if 2014.1.2 is released soon. Otherwise, Helium.


Reply to this email directly or view it on GitHubhttps://github.com//pull/11926#issuecomment-40422149
.

@vortec
Copy link
Contributor Author

vortec commented Apr 15, 2014

@s0undt3ch Here is a test run, the package cannot be found on NPM:

(venv)[vagrant@doge] ~/salt/temp $ ls
(venv)[vagrant@doge] ~/salt/temp $ ../venv/bin/salt-call -c ../venv/etc npm.install testpack registry=http://192.168.2.154:4873 dir=`pwd`
[WARNING ] Although 'dmidecode' was found in path, the current user cannot execute it. Grains output might not be accurate.
[INFO    ] Executing command 'npm --version' in directory '/home/vagrant'
[INFO    ] output: 1.3.14
[INFO    ] Executing command 'npm install --silent --json --registry="http://192.168.2.154:4873" "testpack"' in directory '/home/vagrant/salt/temp'
[INFO    ] stdout: [
  {
    "name": "testpack",
    "version": "0.2.0",
    "from": "testpack@",
    "dependencies": {}
  }
]
local:
    ----------
    - dependencies:
        ----------
    - from:
        testpack@
    - name:
        testpack
    - version:
        0.2.0
(venv)[vagrant@doge] ~/salt/temp $ ls
node_modules
(venv)[vagrant@doge] ~/salt/temp $ ls node_modules
testpack
(venv)[vagrant@doge] ~/salt/temp $

s0undt3ch added a commit that referenced this pull request Apr 15, 2014
npm state module: Add parameter to specify a custom NPM server
@s0undt3ch s0undt3ch merged commit 4f9ebe8 into saltstack:develop Apr 15, 2014
@s0undt3ch
Copy link
Collaborator

Thanks You!

s0undt3ch added a commit to s0undt3ch/salt that referenced this pull request Apr 15, 2014
s0undt3ch added a commit that referenced this pull request Apr 15, 2014
Add `versionadded` to registry support in NPM. Refs #11926.
@vortec vortec deleted the npm_registry branch April 15, 2014 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Execution-Module Feature new functionality including changes to functionality and code refactors, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants