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

PM2 Fix (again): add pm2_env.unique_id checking and cleaning #3626

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

bugsounet
Copy link
Contributor

#3605 : new purpose code with pm2_env.unique_id checking

@rejas rejas merged commit cd6f10c into MagicMirrorOrg:develop Nov 7, 2024
10 checks passed
@FrankBlackMG
Copy link

@bugsounet, I just tested your code changes. All looks good except for this one line.

if (!this.PM2) {

Since this.PM2 is the pm2 index number and it's zero based, you can get the wrong message that PM2 is not in use if MagicMirror happens to be listed first in your managed PM2 process list. Which mine is (and the only).

[2024-11-07 08:55:45.037] [DEBUG] updatenotification: [PM2] found pm2 process id: 0 -- name: mm -- unique_id: b214dbca-7f38-4a3d-9adf-0da9328502a7 
[2024-11-07 08:55:45.037] [INFO]  updatenotification: [PM2] You are using pm2 with id: 0 (mm) 
[2024-11-07 08:55:45.040] [INFO]  updatenotification: [PM2] You are not using pm2 

@sdetweil
Copy link
Collaborator

sdetweil commented Nov 7, 2024

i didn't notice that, i set mine to -1 before looking

@@ -1,7 +1,7 @@
const Exec = require("node:child_process").exec;
const Spawn = require("node:child_process").spawn;
const fs = require("node:fs");

const pm2 = require("pm2");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not happy with moving this to top level.

Problem: With this statement pm2 starts already process(es) in the background so people not using pm2 have this unneeded and unwanted processes running, e.g. in a docker container.

So this should only called after Checking PM2 using as before.

@bugsounet
Copy link
Contributor Author

@khassel : ok, i will reverse it.
@FrankBlackMG : you have right ;)

rejas merges it at lightning speed! (it's my fault I should have drafted it)

rejas pushed a commit that referenced this pull request Nov 9, 2024
continue from #3626 

Is it ok for you ?
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.

5 participants