-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix #3192: Reloading With --update-env Option #5881
base: development
Are you sure you want to change the base?
Conversation
delete opts.env.current_conf.env | ||
// NOTE: `env` gets overriden by `Utility.extendMix` call. Instead, we want to merge envs. | ||
if (opts.env.current_conf.env && typeof(opts.env.current_conf.env) === 'object') | ||
Utility.extendMix(proc.pm2_env.env, opts.env.current_conf.env); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add an proc.pm2_env.env.updateEnv
check here?
if (proc.pm2_env.env.updateEnv) {
Utility.extend(proc.pm2_env.env, opts.env.current_conf.env);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK no, because the env won't be extended with the new variables here:
Line 1362 in 304fc5e
if (update_env === true) { |
Hi, this is my first bug fix for pm2. I've been struggling with the issue described in #3192 for a long time. Previously, I used the following hack to make it work properly:
After reviewing the code, I believe I have found the actual cause of this bug. The environment gets overriden due to a wrong object extension in
Utility.extendExtraConfig
.I'm open to any suggestions for improvements.