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 <pedro.larroy.li...@gmail.com> 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 < > kellen.sunderl...@gmail.com> 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 <lupe...@gmail.com> 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 < > > pedro.larroy.li...@gmail.com > > > > > > > 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. > > > > > > > > > >
