You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2016/12/19 19:07:35 UTC

Re: Review Request 53712: Added `system` environement variables in ` execvpe.cpp`.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53712/#review159636
-----------------------------------------------------------




src/slave/containerizer/mesos/launch.cpp (line 689)
<https://reviews.apache.org/r/53712/#comment230657>

    We should allow system environment variables to be overwritten if they are specified by the framework.  This might cause applications to *not* work, but upon overriding system defaults, it becomes the overidder's problem.
    
    For the MVP for Windows support, I'd argue that overriding system environment variables (with no way to prevent the override) is acceptable.  But we should leave a TODO in the code and a JIRA to consider otherwise.


- Joseph Wu


On Nov. 16, 2016, 10:37 a.m., Daniel Pravat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53712/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2016, 10:37 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added `system` environement variables in ` execvpe.cpp`.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/launch.cpp 320e42748adbabf09f77cb4f5951e2a7ea58fe64 
> 
> Diff: https://reviews.apache.org/r/53712/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>


Re: Review Request 53712: Added `system` environement variables in ` execvpe.cpp`.

Posted by Daniel Pravat <dp...@outlook.com>.

> On Dec. 19, 2016, 7:07 p.m., Joseph Wu wrote:
> > src/slave/containerizer/mesos/launch.cpp, line 689
> > <https://reviews.apache.org/r/53712/diff/2/?file=1565265#file1565265line689>
> >
> >     We should allow system environment variables to be overwritten if they are specified by the framework.  This might cause applications to *not* work, but upon overriding system defaults, it becomes the overidder's problem.
> >     
> >     For the MVP for Windows support, I'd argue that overriding system environment variables (with no way to prevent the override) is acceptable.  But we should leave a TODO in the code and a JIRA to consider otherwise.

MESOS-6816 has been opened to track the overwrite.


- Daniel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53712/#review159636
-----------------------------------------------------------


On Nov. 16, 2016, 6:37 p.m., Daniel Pravat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53712/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added `system` environement variables in ` execvpe.cpp`.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/launch.cpp 320e42748adbabf09f77cb4f5951e2a7ea58fe64 
> 
> Diff: https://reviews.apache.org/r/53712/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>