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: docker support #38

Merged
merged 54 commits into from
Mar 8, 2018
Merged

feat: docker support #38

merged 54 commits into from
Mar 8, 2018

Conversation

thonatos
Copy link
Member

@thonatos thonatos commented Mar 6, 2018

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
  • package.json
Description of change
  • add script to bootstrap egg without deamon
  • add dockerfile
  • add docker-compose file to start egg with redis and mongodb
  • update config.local.js with mongo/redis

@codecov
Copy link

codecov bot commented Mar 6, 2018

Codecov Report

Merging #38 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #38   +/-   ##
=======================================
  Coverage   59.17%   59.17%           
=======================================
  Files          34       33    -1     
  Lines         926      926           
=======================================
  Hits          548      548           
  Misses        378      378
Impacted Files Coverage Δ
config/config.default.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 e3304c4...cf796ee. Read the comment docs.

@thonatos thonatos mentioned this pull request Mar 6, 2018
2 tasks
atian25
atian25 previously requested changes Mar 7, 2018

exports.redis = {
client: {
host: process.env.EGG_REDIS_HOST || '127.0.0.1',
Copy link
Member

Choose a reason for hiding this comment

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

有 prod 的话,unittest 和 local 里面的配置,可以考虑写到 default 去

Copy link
Member

Choose a reason for hiding this comment

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

不过都有环境变量了,那是不是 prod 也不需要了

Copy link
Member

Choose a reason for hiding this comment

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

还是不要搞那么多环境变量吧,弄个配置文件更好。

Copy link
Member Author

@thonatos thonatos Mar 7, 2018

Choose a reason for hiding this comment

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

这边的加 prod 的原因写在 Dockerfile 的 review 里了

Copy link
Member Author

Choose a reason for hiding this comment

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

issue #28 讨论吧

})

db.egg_cnode.insert({
'cnode': 'egg-cnode'
Copy link
Member

Choose a reason for hiding this comment

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

  • cnode 没有 ''
  • 这个文件可以不用 ignore 掉 eslint 的,在顶部加一句注释,加个 /* global db */ 就 ok 了

Copy link
Member Author

@thonatos thonatos Mar 7, 2018

Choose a reason for hiding this comment

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

👌, 加了 /* eslint-disable */


# RUN npm i --production

RUN npm i --production --registry=https://registry.npm.taobao.org
Copy link
Member

Choose a reason for hiding this comment

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

我的想法是,本地开发给一个 docker 来启动数据库等环境,本地应该不需要在 docker 里面复制源码和 npm

Copy link
Member Author

@thonatos thonatos Mar 7, 2018

Choose a reason for hiding this comment

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

唔,感觉可以加一个 compose 配置:

  • docker-compose.yml 启动一个基本和正式环境完全一致的服务(egg/redis/mongo)
  • docker-compose-dev.yml 启动数据库等

Copy link
Member

Choose a reason for hiding this comment

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

我觉得 docker 支持作为另一个 repo 吧。

Copy link
Member

Choose a reason for hiding this comment

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

我现在的需求很简单,要提供开发期支持,不然本地 egg-bin test 的时候,要自己启动 redis 和 mongo

部署这个暂时不关心

Copy link
Member Author

Choose a reason for hiding this comment

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

加了一个 docker-compose.dev.yml 来启动数据库

@atian25
Copy link
Member

atian25 commented Mar 7, 2018

rebase

@thonatos
Copy link
Member Author

thonatos commented Mar 7, 2018

@atian25 唔,配置这边有点改动,
使用 docker 启动的 redis/mongo 加了认证,同时改变了数据库名称,统一使用 egg_cnode

@JacksonTian
Copy link
Member

没有rebase 对吧。

@thonatos thonatos dismissed atian25’s stale review March 8, 2018 15:03

config.prod.js has been removed.

@atian25 atian25 merged commit ce9247a into master Mar 8, 2018
@atian25 atian25 deleted the feature-docker branch March 8, 2018 15:06
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.

3 participants