-
Notifications
You must be signed in to change notification settings - Fork 56
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: supports serialization of options #97
Closed
Closed
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
59e8612
feat: supports serialization of options
waitingsong 8aa9fc2
chore: fix style
waitingsong 23fc7b3
chore: move option.serialization of validation into utils/options.js
waitingsong 9d9f33a
refactor: update validation of options.serialization
waitingsong 9727bf3
chore: update parameter comments of class Master
waitingsong 097dd0a
chore: clean README.md
waitingsong 1249d64
feat: update Node.js version requirement for serialization
waitingsong File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,8 @@ class Master extends EventEmitter { | |
* - {Object} [https] https options, { key, cert, ca }, full path | ||
* - {Array|String} [require] will inject into worker/agent process | ||
* - {String} [pidFile] will save master pid to this file | ||
* - {json|advanced} [serialization] Advanced serialization for IPC since Node.js v12.16.0, | ||
* see: https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V12.md#advanced-serialization-for-ipc | ||
*/ | ||
constructor(options) { | ||
super(); | ||
|
@@ -248,6 +250,14 @@ class Master extends EventEmitter { | |
const debugPort = process.env.EGG_AGENT_DEBUG_PORT || 5800; | ||
if (this.options.isDebug) opt.execArgv = process.execArgv.concat([ `--${semver.gte(process.version, '8.0.0') ? 'inspect' : 'debug'}-port=${debugPort}` ]); | ||
|
||
/* istanbul ignore next */ | ||
if (semver.gte(process.version, '12.16.0')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里的判断有一个问题,'v13.0.0' 也是 >= 12.16.0,但是这个版本 patch 了此功能么 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 对于 12 版本是 >= 12.16.0, 对于 13 版本是 >= 13.2.0。 |
||
/* istanbul ignore next */ | ||
if (this.options.serialization) { | ||
opt.serialization = this.options.serialization; | ||
} | ||
} | ||
|
||
const agentWorker = childprocess.fork(this.getAgentWorkerFile(), args, opt); | ||
agentWorker.status = 'starting'; | ||
agentWorker.id = ++this.agentWorkerIndex; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
这里是 12.16.0 吧?
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.
我也是如此考虑:随着版本更新性能提升及可靠性得到进一步验证,可能在未来某个版本默认改为 advanced。
所以跟随 node.js 默认设置(json),代码里面不做默认值设置,让有需要的用户自行决定传入参数控制。