feat: [BREAKING_CHANGE] check the status of app when start on daemon by popomore · Pull Request #9 · eggjs/scripts

popomore · 2017-10-12T08:47:43Z

Checklist

npm test passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

Description of change

check whether stderr contains log
check starting time, the maximum is 300s
wait app start and exit
drop Windows support

1. check whether stderr contains log 1. check starting time, the maximum is 300s 1. wait app start and exit

atian25 · 2017-10-12T08:53:24Z

 };

- this.logger.info(`starting egg application at ${baseDir}`);
+ this.logger.info(`Starting egg application at ${baseDir}`);


哈，我之前觉得大写不好看，特意改为小写的。

atian25 · 2017-10-12T08:57:02Z

+ while (true) {
+ const stat = yield fs.stat(stderr);
+ if (stat && stat.size > 0) {
+ const [ stdout ] = yield exec('tail -n 100 ' + stderr);


start 之前是支持 win 的，这样改后就不支持了吧？

https://github.com/lucagrulla/node-tail 倒是有个库

话说，换个思路的话，master 有错误的时候， emit 一个 message 给 parent ？

这个不是功能性的，是体验优化。可以在部署日志里面大致看到错误，而不用上服务器看日志。
master 也没有 app 和 agent 的错误，不知道是谁打的。

这里不是会 kill master 么? 不仅仅是体验优化了吧?

atian25 · 2017-10-12T08:59:32Z

+ const [ stdout ] = yield exec('tail -n 100 ' + stderr);
+ this.logger.error(stdout);
+ this.logger.error('Start failed, see %s', stderr);
+ this.child.kill('SIGTERM');


master 只要有 error 日志，就 kill 掉？

对，不能打到 stderr 里

warning 日志是在 stdout 吧？
v8 debug 的那个启动日志好像是会跑去 stderr 的，不过应该不会用这个来 debug

console.warn 到时到 stderr 的

atian25 · 2017-10-12T09:00:02Z

+ if (this.isReady) return;
+
+ if (count >= timeout) {
+ // 启动超时，提示查看 chair-web.log 日志


codecov-io · 2017-10-12T09:01:39Z

Codecov Report

Merging #9 into master will increase coverage by 1.07%.
The diff coverage is 100%.

@@ Coverage Diff @@## master #9 +/- ##
==========================================
+ Coverage 97.7% 98.78% +1.07% 
==========================================
Files 6 6 Lines 131 165 +34 ==========================================
+ Hits 128 163 +35 + Misses 3 2 -1

Impacted Files	Coverage Δ
lib/cmd/start.js	`100% <100%> (ø)`	⬆️
lib/cmd/stop.js	`93.33% <0%> (+3.33%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfd0d2f...a3ce0df. Read the comment docs.

popomore · 2017-10-12T09:41:14Z

@atian25 还有哪些要改的

popomore · 2017-10-12T09:46:03Z

README 不是写着 windows 不支持吗

popomore · 2017-10-12T10:25:13Z

starting 的时候最好能取到 framework 的名字

atian25 · 2017-10-12T10:27:30Z

starting 的时候最好能取到 framework 的名字

不是有 getFramework 了么? 读下 package.json 然后 print 下?

README 不是写着 windows 不支持吗

只是写着 stop 不支持吧？

如果 start 也不支持的话，README 和 egg docs 都要改下。

+1

popomore · 2017-10-12T14:11:03Z

@atian25 这个需要发 break 吗

atian25 · 2017-10-12T14:11:04Z

 deploy tool for egg project.

+**Note: Windows is not supported**
+


算不算 break？之前好像有人在 win 使用。

atian25 · 2017-10-12T14:13:13Z

+ // nothing
+ }
+ if (stat && stat.size > 0) {
+ const [ stdout ] = yield exec('tail -n 100 ' + stderr);


要不然这里兼容下，win 打个 warning 提示不再兼容，跳过这段逻辑

那我把这里去掉吧

atian25 · 2017-10-12T14:30:47Z

 });
+
+ // check start status
+ yield this.checkStatus(argv);


这里直接用 getRotatelog(argv.stderr) 返回的流？就不用读文件了？

哪里读文件了？

popomore · 2017-10-12T14:48:08Z

那问题可以合并了

atian25 · 2017-10-13T02:10:29Z

+ timeout: {
+ description: 'a timeout for start when daemon',
+ type: 'number',
+ default: 300,


改为 ms 符合直觉一点吧? 下面自己除掉.
其他 OK，可以合并

popomore · 2017-10-13T02:58:20Z

改了

atian25 · 2017-10-13T05:22:28Z

你发 major 吧，然后 egg docs 那边也要改下

popomore · 2017-10-13T14:07:55Z

egg-scripts@2.0.0

feat: check the status of app when start on daemon
2cbc410
1. check whether stderr contains log 1. check starting time, the maximum is 300s 1. wait app start and exit

popomore requested a review from atian25 October 12, 2017 08:47

popomore added 2 commits October 12, 2017 16:54

f
05386bd

f
e83592a

atian25 approved these changes Oct 12, 2017
View reviewed changes

popomore added 2 commits October 12, 2017 17:02

f
cc465dc

f
5e773be

popomore added 3 commits October 12, 2017 20:52

f
bc8dbe9

f
ded4b50

f
9235c39

atian25 reviewed Oct 12, 2017
View reviewed changes

f
3a11919

popomore force-pushed the status branch from fe5ee66 to 3a11919Compare October 13, 2017 01:56

popomore changed the title ~~feat: check the status of app when start on daemon~~feat: [BREAKING_CHANGE] check the status of app when start on daemonOct 13, 2017

atian25 reviewed Oct 13, 2017
View reviewed changes

f
a3ce0df

atian25 merged commit 0f7ca50 into masterOct 13, 2017

atian25 deleted the status branch October 13, 2017 05:20

		deploy tool for egg project.

		Note: Windows is not supported

Uh oh!

Conversation

popomore commented Oct 12, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Oct 12, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

popomore commented Oct 12, 2017

Uh oh!

popomore commented Oct 12, 2017

Uh oh!

popomore commented Oct 12, 2017

Uh oh!

atian25 commented Oct 12, 2017

Uh oh!

popomore commented Oct 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

popomore commented Oct 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

popomore commented Oct 13, 2017

Uh oh!

atian25 commented Oct 13, 2017

Uh oh!

popomore commented Oct 13, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

popomore commented Oct 12, 2017•
edited
Loading

codecov-io commented Oct 12, 2017•
edited
Loading