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

Don't do nodejs::install when manage_package_repo is set to false. #478

Closed

Conversation

anordby
Copy link

@anordby anordby commented Jul 17, 2023

Pull Request (PR) description

Do not call the nodejs::install when manage_package_repo is set to false.

This Pull Request (PR) fixes the following issues

When/if you like to write /root/.npmrc yourself, it will fail otherwise as nodejs module also does it:

Jul 17 10:10:25 no000010sresd0 puppet-agent[31736]: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: File[/root/.npmrc] is already declared at (file: /etc/puppetlabs/code/vcs/ops/ops_master/external-modules/nodejs/manifests/install.pp, line: 55); cannot redeclare (file: /etc/puppetlabs/code/vcs/devops/devops_master/modules/moller/manifests/modules/node.pp, line: 42) (file: /etc/puppetlabs/code/vcs/devops/devops_master/modules/moller/manifests/modules/node.pp, line: 42, column: 2) (file: /etc/puppetlabs/code/vcs/devops/devops_master/modules/moller/manifests/modules/mollerservice.pp, line: 85) on node no000010sresd0.moller.local

@kenyon kenyon force-pushed the no-install-package-manage-off branch from ac0b19c to 37bb38a Compare September 16, 2023 22:53
@kenyon kenyon force-pushed the no-install-package-manage-off branch from 37bb38a to 44b278b Compare September 21, 2023 04:22
@anordby
Copy link
Author

anordby commented Nov 28, 2023

Can I have some action here @evgeni ?

@anordby
Copy link
Author

anordby commented Nov 28, 2023

Or @kenyon ?

@smortex
Copy link
Member

smortex commented Nov 28, 2023

Not a user of nodejs so maybe I am missing something obvious, but…

Indeed this break CI. The point of a nodejs module is to manage nodejs… Skipping the repository may be necessary for some users (in order to use a custom one I suppose) but skipping package installation and configuration does not make sense IMHO.

My guess is that your moller module should leverage the nodejs module to make it generate the configuration you expect.

@anordby
Copy link
Author

anordby commented Nov 28, 2023

I agree the checks should be fixed of course. Can have a look. But purpose of manage_package_repo should be to not manage repo or package? If package repo is not managed how can you install? Some of just want/wanted the npm provider. Did not get any feedback so far, that's why I did not do anything more.. @smortex

@evgeni
Copy link
Member

evgeni commented Nov 28, 2023

If you just need the provider, you don't need to include the module at all as long as it's available in the environment.

@kenyon kenyon changed the title Don't do nodejs::install when manage_package_repo is set to false. Don't do nodejs::install when manage_package_repo is set to false. Nov 28, 2023
@kenyon
Copy link
Member

kenyon commented Nov 28, 2023

Agree with @smortex and @evgeni and IMHO we should close this. Maybe instead we could document in the README that the npm package provider can be used without including the npm class.

@evgeni evgeni added the invalid This doesn't seem right label Jan 3, 2024
@evgeni
Copy link
Member

evgeni commented Jan 3, 2024

Closing as invalid, feel free to reopen when you have more details.

@evgeni evgeni closed this Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants