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

添加缓存机制:将已经提交成功的文件保存,下次发布则不会重复提交。 #5

Closed
wants to merge 2 commits into from

Conversation

zswang
Copy link

@zswang zswang commented Dec 5, 2015

并考虑是否有 hash 的情况

@2betop
Copy link
Contributor

2betop commented Dec 5, 2015

这个赞

@2betop
Copy link
Contributor

2betop commented Dec 5, 2015

待我周一的时候体验一下,然后发版本,感谢大叔 :)

@hefangshi
Copy link
Member

几点疑问

  1. 这个功能的需求是为了解决 可否增加 增量发布的功能 #1 这样的需求么?需求中描述的增量部署、网络环境差、以及希望用重试等手段保证部署的完备性等等,给我的感觉是这样的场景是用 http-push 来做生产环境部署?但是使用 http-push 这样的插件去做生产环境的部署,即使不提分布式架构下的部署瓶颈,在安全性和完备性上也有非常非常严重的问题,这个功能是不应该用于这样的场景的。
  2. 缓存机制功能是只要配置了 cacheDir 就开启么?那么如果用户希望全量部署的时候也无法通过调整命令行参数去全量部署,而必须修改配置才能进行全量部署,进而导致编译缓存的失效。因此我的建议是不要使用参数配置而是使用命令行参数来控制缓存行为。
  3. cacheDir的命名也会有很多歧义,cacheDeploy应该更贴切一些?设置缓存的目录的必要性也不高,因为可以随FIS的整体缓存目录调整,设置是否开启部署缓存会更直观一些。
  4. mkdirp这类三方扩展应该尽量少的加入,fis核心中已经有 fis.util.mkdir 这样的 API 实现一样的功能了。
  5. 似乎没有缓存过期机制?这个文件是否会越来越大,遍历速度越来越慢?

在常规开发时,一次全量部署的时间在我们自己体验来看并不是很长,添加部署缓存容易造成更多的不确定性。所以并没有作为痛点问题去解决。

总结一下,加这样的功能我觉得有2个基准要考虑

  1. 需求不能是用于生产环境部署,应该有更正确的使用场景
  2. 默认应该是不开启的,可以通过配置启动,并且可以通过命令行参数关闭

@zswang
Copy link
Author

zswang commented Dec 7, 2015

需求不能是用于生产环境部署,应该有更正确的使用场景

如果只考虑「测试机(联调机)」的场景,确实没有必要做这种标记,因为网速可以保证。
应用到生产环境就另说了。

涉及安全性和完备性,必须要改造接收端。

默认应该是不开启的,可以通过配置启动,并且可以通过命令行参数关闭

改动一个插件的配置,相比改动 fis 的命令行,前者当然更容易被接受。

为保证 FIS 的纯洁,我另创建一个插件好了。

@zswang zswang closed this Dec 7, 2015
@hefangshi
Copy link
Member

建议如果是生成环境部署,即使是用 http-push ,也还是用tar包部署吧,一次性传输,无论是重试还是校验都方便的多。类似 fis1那样的,在接收端解压。

部署其实可以考虑下 git 的模式呀,很多公司都这样用,小规模效果不错。

@oxUnd
Copy link
Contributor

oxUnd commented Dec 7, 2015

赞美

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.

4 participants