-
Notifications
You must be signed in to change notification settings - Fork 37
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
add ls method to list egg runing app #31
base: master
Are you sure you want to change the base?
Conversation
@fengmk2 pm2 code can use in egg? |
lib/monitor.js
Outdated
|
||
const pidusage = require('pidusage'); | ||
|
||
exports.getMonitorData = function getMonitorData(processs, cb) { |
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.
don't use callback
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.
ok, now without callback
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.
Unit test is required
const { argv } = context; | ||
this.logger.info(`list egg application ${argv.title ? `with --title=${argv.title}` : ''}`); | ||
|
||
const processList = yield this.helper.findNodeProcessWithPpid(item => { |
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.
You can move the filter into findNodeProcessWithPpid.
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.
@popomore This filter function use a title variable , so I think it's more appropriate to put it here.
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.
maybe this could extract to a utils function, and share with stop cmd
Codecov Report
@@ Coverage Diff @@
## master #31 +/- ##
===========================================
- Coverage 99.51% 57.66% -41.86%
===========================================
Files 6 9 +3
Lines 207 385 +178
===========================================
+ Hits 206 222 +16
- Misses 1 163 +162
Continue to review full report at Codecov.
|
const path = require('path'); | ||
const Command = require('../command'); | ||
const isWin = process.platform === 'win32'; | ||
const osRelated = { |
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.
it's duplication with stop cmd, extract this to helper or?
} | ||
|
||
get description() { | ||
return 'ls app'; |
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.
list egg process
item.isAgent = false; | ||
item.isWorker = false; | ||
|
||
let tileFlag = true; |
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.
tileFlag -> titleFlag
const { argv } = context; | ||
this.logger.info(`list egg application ${argv.title ? `with --title=${argv.title}` : ''}`); | ||
|
||
const processList = yield this.helper.findNodeProcessWithPpid(item => { |
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.
maybe this could extract to a utils function, and share with stop cmd
} | ||
return false; | ||
}); | ||
try { |
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.
new line
} | ||
|
||
|
||
// get tile string in the script, tile as the project name ? |
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.
title
|
||
exports.getMonitorData = function* getMonitorData(processs) { | ||
|
||
return new Promise(function(resolve, reject) { |
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.
prefer arrow style
return; | ||
} | ||
|
||
const pids = processs.map(pro => { |
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.
const pids = processs.map(p => p.pid)
resolve(pids); | ||
return; | ||
} | ||
pidusage(pids, function(err, statistics) { |
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.
prefer arrow style
console.error('Error caught while calling pidusage'); | ||
console.error(err); | ||
|
||
processs.map(function(pro) { |
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.
prefer arrow style
ping |
@atian25 I’m a bit busy recently, I’ll try to fix the bug this month. |
没关系,有时间再改就行,不急。只是顺手 ping 下看看是不是哪里卡住了。 |
@@ -0,0 +1,145 @@ | |||
'use strict'; | |||
// code copy from pm2 |
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.
PM2 是 AGPL 协议的,这里还是自己实现吧,不要复制,也不要参考太多。
Description of change
This command can output a table in the shell, list all the running app.
Egg-scripts does not have such a command, this commit add this command, can output a table to list the running egg app.
when install egg-scripts with global, just use
eg:
can list all egg runing apps
It can add a title option to filter apps.
eg:
This will filter the title include xxxx string apps.
Note:
egg-scripts
is not recommended to install global, you should install and use it as npm scripts. so can add it in npm scripts.Most of the code is copied from the list function in pm2.