Skip to content

feat: startCluster return master #83

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: startCluster return master #83

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44af285
feat: startCluster return master
leoner Nov 20, 2018
a4cab3a
Merge branch 'master' into feat-return-master
Mar 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ const Master = require('./lib/master');
* @method Egg#startCluster
* @param {Object} options {@link Master}
* @param {Function} callback start success callback
* @return {Object} - master
*/
exports.startCluster = function(options, callback) {
new Master(options).ready(callback);
const master = new Master(options);
master.ready(callback);
return master;
Copy link
Author

Choose a reason for hiding this comment

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

就是当初这里设计的时候, 没有把 master 返回, 有啥原因?

Copy link
Author

Choose a reason for hiding this comment

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

目前遇到一个场景, 就是需要在插件中启动一个 app.

return eggServer.startCluster({
      workers: 1,
      port: serverPort,
      baseDir: __dirname,
    });

现在这个地方没有返回, 导致用例结束的时候, master退出的时候, 打印的日志是 error 不太友好.

image

Copy link
Member

Choose a reason for hiding this comment

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

这里其实应该返回 promise

Copy link
Author

Choose a reason for hiding this comment

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

恩, 就是返回一个可以主动关闭 master的句柄就行呀. 这样如果在代码启动一个 server, 也可以主动的去关闭, 要不现在没有地方去关.只能等进程退出, 这样也会提示 error 不太友好.
如果主动退出, 就会好很多.
image

Copy link
Member

Choose a reason for hiding this comment

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

插件里面启动另一个 egg？

Copy link
Member

Choose a reason for hiding this comment

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

嗯，之前的做法是启动一个子进程，再杀掉。

这里 exports 还不够，上层框架也要 exports

Copy link
Member

Choose a reason for hiding this comment

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

datahub 是不是用单进程启动就好了

Copy link
Author

Choose a reason for hiding this comment

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

恩, 我现在遇到的这个场景是可以了
https://github.com/macacajs/macaca-datahub/blob/master/index.js#L57, 他是这样用的.

那看看这样如果没有啥问题了, 可以先合并了. 我先用着呀.

Copy link
Author

Choose a reason for hiding this comment

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

嗯，之前的做法是启动一个子进程，再杀掉。

之前他们可能就是没有办法杀掉, 就用了子进程的方式, 结果还采坑了 eggjs/egg-datahub#9 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

那是他们那边子进程没用对吧 😂

exports 这个我没问题，看贯高。

};