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

feat: support boot lifecyle #2972

Merged
merged 5 commits into from
Sep 12, 2018
Merged

feat: support boot lifecyle #2972

merged 5 commits into from
Sep 12, 2018

Conversation

killagu
Copy link
Contributor

@killagu killagu commented Sep 6, 2018

Refs: #2520

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

loader

Description of change

support load boot hook

@killagu killagu requested review from atian25 and popomore and removed request for atian25 September 6, 2018 14:02
@codecov
Copy link

codecov bot commented Sep 6, 2018

Codecov Report

Merging #2972 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2972      +/-   ##
==========================================
+ Coverage   99.75%   99.75%   +<.01%     
==========================================
  Files          29       29              
  Lines         826      830       +4     
==========================================
+ Hits          824      828       +4     
  Misses          2        2
Impacted Files Coverage Δ
lib/egg.js 100% <100%> (ø) ⬆️
lib/loader/agent_worker_loader.js 100% <100%> (ø) ⬆️
lib/loader/app_worker_loader.js 100% <100%> (ø) ⬆️

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 b02ce15...614c194. Read the comment docs.

`beforeClose` 注册方法在 app/agent 实例的 `close` 方法被调用后, 按注册的逆序执行。一般用于资源的释放操作, 例如 [`egg`](https://github.com/eggjs/egg/blob/master/lib/egg.js) 用来关闭 logger , 删除监听方法等。

__这个方法不建议在生产环境使用, 可能遇到未执行完就结束进程的问题。__
Egg提供了配置文件加载完成(`configDidLoad`), 文件加载完成(`didLoad`), 插件启动完毕(`willReady`), worker 准备就绪(`didReady`), 应用启动完成(`serverDidReady`), 应用即将关闭(`beforeClose`)这些生命周期函数。
Copy link
Member

Choose a reason for hiding this comment

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

  • Egg -> 框架


app before start 100ms: 147.546ms
app before start 200ms: 245.405ms // 并行执行
使用 `beforeClose` 的时候需要注意,在 egg 的 进程关闭处理中是有超时时间的,如果 worker 进程在接收到进程退出信号之后,没有在所规定的时间内退出,将会被强制关闭。
Copy link
Member

Choose a reason for hiding this comment

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

在 egg 的 进程关闭 -> 在框架的进程关闭


app ready
Deprecate methods:
Copy link
Member

Choose a reason for hiding this comment

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

  • 中文
  • 另外,既然是 Deprecate,那他们分别的替代方式,可以描述下

console.timeEnd('app before close 100ms');
});
// app.js or agent.js
class AppBootHook {
Copy link
Member

Choose a reason for hiding this comment

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

这里没有说明清楚,定义这个类能干嘛,然后在应用层如何给挂载给 egg

```

开发者使用类的方式定义 `app.js` 和 `agent.js` 之后, 框架会自动加载并实例化这个类, 并且在各个生命周期阶段调用对应的方法。
Copy link
Member

@atian25 atian25 Sep 7, 2018

Choose a reason for hiding this comment

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

那之前在里面操作 app.coreMiddleware 的应该放在哪个 method 里面?

是不是写个 old app.js 和 new app.js 的对比

Copy link
Contributor Author

Choose a reason for hiding this comment

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

应该要放在 configDidLoad 里面

Copy link
Contributor Author

Choose a reason for hiding this comment

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

具体还是看我贴的那个图比较清楚。文字描述比较困难。

@atian25 atian25 mentioned this pull request Sep 11, 2018
4 tasks
@atian25
Copy link
Member

atian25 commented Sep 11, 2018

修复下


agent ready
启用的方法:
Copy link
Member

Choose a reason for hiding this comment

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

这里应该说明是 Migration 吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️错别字。本来想写的是 弃用的方法。

@killagu
Copy link
Contributor Author

killagu commented Sep 11, 2018

@atian25 合了吧?

@atian25 atian25 changed the title Support boot feat: support boot lifecyle Sep 12, 2018
@atian25 atian25 merged commit 0d876c7 into master Sep 12, 2018
@atian25 atian25 deleted the support-boot branch September 12, 2018 00:56
popomore pushed a commit that referenced this pull request Sep 12, 2018
feat: support boot lifecyle (#2972)

Refs: #2520
@atian25
Copy link
Member

atian25 commented Sep 12, 2018

其实放到 https://eggjs.org/zh-cn/basics/app-start.html 才合适,回头我看看怎么写吧

@atian25
Copy link
Member

atian25 commented Sep 12, 2018

另外,这个文档对应的英文版也需要更新

@ghost ghost mentioned this pull request Sep 12, 2018
4 tasks
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.

None yet

3 participants