I agree with Kellen on not renaming the CI docker files (by renaming - i think its implicit you can use these for production) i don't think we should telling our users go use these bloated docker files, you could create lean separate docker files for production use-case with only necessary runtime packages.
-1 On Wed, Oct 17, 2018 at 11:48 AM kellen sunderland < [email protected]> wrote: > Hey Pedro, sorry I still don't see a good reason to justify changing the > filenames. Renaming them to be less specific isn't going to explain to > users what the purpose of the files is, and it could cause breakages with > any system that refer to these files including external company's CI > systems. If I think of the benefits versus potential errors introduced by > making the change I see more potential risk than obvious benefits. I also > feel that this change will make the difference between the runtime docker > files and the CI docker files less clear to users, not more clear. In > general I think adding a descriptive README.md would server our purposed > better here. Happy to hear what others think. > > On Wed, Oct 17, 2018 at 6:45 AM Pedro Larroy <[email protected] > > > wrote: > > > Hi Kellen, thank you for your response. > > > > Maybe I didn't explain myself correctly. The purpose of this > infrastructure > > is not changed. > > > > I'm not planning to use these Dockerfiles as MXNet docker containers for > > users to run MXNet, that is a separate concern. > > > > It is just that some of this Dockerfiles we use in CI to build, test and > > generate documentation, so are used as a runtime container as well. Thus > > i'm just changing the pathing for semantic reasons and remove the .build. > > which is just noise. > > > > As an example I would like to explain that we are about to merge the PR > > which uses QEMU to run the unit tests, so there's an associated > Dockerfile > > which hosts the QEMU runtime environment used to execute the unit tests > in > > an ARM emulated machine. Thus makes little sense that these Dockerfiles > are > > called "build". I don't know if my explanation changes your vote. Either > > way please let me know. Separating this change in a different PR was > > suggested by several MXNet contributors during review. > > > > Pedro. > > > > On Wed, Oct 17, 2018 at 11:21 AM kellen sunderland < > > [email protected]> wrote: > > > > > -1. (non-binding) > > > > > > These Dockerfiles are very bloated and imo only useful for creating a > > build > > > environment or running tests. Just as you wouldn't setup a server for > a > > > service and then install 200 packages that may or may not be used for > the > > > service I wouldn't recommend using these Dockerfiles at runtime. > Runtime > > > Dockerfiles should in my opinion be as lightweight and suited to their > > task > > > as possible. > > > > > > On Wed, Oct 17, 2018, 1:58 AM Hagay Lupesko <[email protected]> wrote: > > > > > > > The PR provides a good explanation of this change and all code > updates. > > > > LGTM. > > > > > > > > On Tue, Oct 16, 2018 at 8:41 AM Pedro Larroy < > > > [email protected] > > > > > > > > > wrote: > > > > > > > > > Hi > > > > > > > > > > I would like to rename the dockerfiles since they are used as a > > runtime > > > > > environment and not only as build as they were initially intended. > > > > > > > > > > More info about the change in this PR: > > > > > https://github.com/apache/incubator-mxnet/pull/12423/files > > > > > > > > > > > > > > > Pedro. > > > > > > > > > > > > > > >
