Re: [LAZY VOTE]: rename dockerfiles s/.build.//
Allright then, I closed that PR. Thanks for your feedback. On Wed, Oct 17, 2018 at 9:06 PM kellen sunderland < kellen.sunderl...@gmail.com> wrote: > May be of interest to people that we're trying get a good set of > production-ready Dockerfiles (which I'm referring to as runtime Dockerfiles > in this thread) with a PR open here: > https://github.com/apache/incubator-mxnet/pull/12791 (thanks for updating > these Meghna). > > On Wed, Oct 17, 2018 at 12:00 PM Naveen Swamy wrote: > > > 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 < > > kellen.sunderl...@gmail.com> 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 < > > 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 > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: [LAZY VOTE]: rename dockerfiles s/.build.//
May be of interest to people that we're trying get a good set of production-ready Dockerfiles (which I'm referring to as runtime Dockerfiles in this thread) with a PR open here: https://github.com/apache/incubator-mxnet/pull/12791 (thanks for updating these Meghna). On Wed, Oct 17, 2018 at 12:00 PM Naveen Swamy wrote: > 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 < > kellen.sunderl...@gmail.com> 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 < > 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 > 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. > > > > > > > > > > > > > > > > > > > > >
Re: [LAZY VOTE]: rename dockerfiles s/.build.//
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 < kellen.sunderl...@gmail.com> 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 > > 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 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. > > > > > > > > > > > > > > >
Re: [LAZY VOTE]: rename dockerfiles s/.build.//
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 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 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. > > > > > > > > > >
Re: [LAZY VOTE]: rename dockerfiles s/.build.//
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 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. > > > > > >
Re: [LAZY VOTE]: rename dockerfiles s/.build.//
-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 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 > > 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. > > >
Re: [LAZY VOTE]: rename dockerfiles s/.build.//
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 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. >
[LAZY VOTE]: rename dockerfiles s/.build.//
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.
