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

Map node binaries whenever rvm1:hook is called and evaluate the fnm_setup_command dynamically #5303

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

Senen
Copy link
Member

@Senen Senen commented Oct 24, 2023

References

The previous version worked fine when using Capistrano to deploy a new release, but it did not work for other commands, like cap npm install or cap puma:start, as they do not create a release. Therefore, the git:create_release was never called, and the map_node_bins was not invoked.

Objectives

Since we need node binaries available to the deploy user when executing any process that runs the application, like puma, delayed_job, and rake tasks, among others, it makes sense to load FNM to use the correct Node version in the same places where Capistrano loads RVM to load the correct Ruby version.

With dynamic loading of the fnm_setup_command, we get the correct path when deploying a new release and also when running any other capistrano command that does not deploy a new release.

…m_setup_command` dynamically

The previous version worked fine when using Capistrano to deploy a new
release, but it did not work for other commands, like `cap npm install`
or `cap puma:start`, as they do not create a release. Therefore, the
`git:create_release` was never called, and the `map_node_bins` was not
invoked.

Since we need node binaries available to the deploy user when executing
any process that runs the application, like puma, delayed_job, and rake
tasks, among others, it makes sense to load FNM to use the correct Node
version in the same places where Capistrano loads RVM to load the correct
Ruby version.

With dynamic loading of the `fnm_setup_command`, we get the correct path
when deploying a new release and also when running any other capistrano
command that does not deploy a new release.
@javierm javierm added this to Reviewing in Consul Democracy Oct 24, 2023
Copy link
Member

@javierm javierm left a comment

Choose a reason for hiding this comment

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

Superb catch! 👏 So great we've detected this case before releasing a new version 😄.

Consul Democracy automation moved this from Reviewing to Testing Oct 24, 2023
@Senen Senen merged commit f36248f into master Oct 24, 2023
15 checks passed
Consul Democracy automation moved this from Testing to Release 2.1.0 Oct 24, 2023
@Senen Senen deleted the fix_fnm_setup branch October 24, 2023 13:24
@javierm javierm self-assigned this Oct 24, 2023
@Senen Senen mentioned this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants