[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[ https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14551599#comment-14551599 ] Benjamin Mahler commented on MESOS-994: --- [~tnachen] I saw you reviewed the first one, can you review the rest as well? :) Add an Optionstring os::getenv() to stout --- Key: MESOS-994 URL: https://issues.apache.org/jira/browse/MESOS-994 Project: Mesos Issue Type: Improvement Components: stout, technical debt Reporter: Ian Downes Assignee: Greg Mann Labels: newbie This would replace the common pattern of: Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None() -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[ https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14546925#comment-14546925 ] Greg Mann commented on MESOS-994: - Review requests submitted. We have Ben's original patch for os::getenv(): https://reviews.apache.org/r/13978/ along with changes to callers in /src, libprocess, and stout, respectively: https://reviews.apache.org/r/34317/ https://reviews.apache.org/r/34318/ https://reviews.apache.org/r/34319/ Add an Optionstring os::getenv() to stout --- Key: MESOS-994 URL: https://issues.apache.org/jira/browse/MESOS-994 Project: Mesos Issue Type: Improvement Components: stout, technical debt Reporter: Ian Downes Assignee: Greg Mann Labels: newbie This would replace the common pattern of: Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None() -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[ https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14546409#comment-14546409 ] Greg Mann commented on MESOS-994: - I have a patch for this, but lack permissions to modify https://reviews.apache.org/r/13979/. Can I be granted permissions to modify that review request? Add an Optionstring os::getenv() to stout --- Key: MESOS-994 URL: https://issues.apache.org/jira/browse/MESOS-994 Project: Mesos Issue Type: Improvement Components: stout, technical debt Reporter: Ian Downes Assignee: Greg Mann Labels: newbie This would replace the common pattern of: Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None() -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[ https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14546419#comment-14546419 ] Benjamin Mahler commented on MESOS-994: --- Go ahead and open a new review request and I'll discard that one. Add an Optionstring os::getenv() to stout --- Key: MESOS-994 URL: https://issues.apache.org/jira/browse/MESOS-994 Project: Mesos Issue Type: Improvement Components: stout, technical debt Reporter: Ian Downes Assignee: Greg Mann Labels: newbie This would replace the common pattern of: Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None() -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[
https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14531189#comment-14531189
]
Benjamin Mahler commented on MESOS-994:
---
Those are possibly to deal with the poor semantics of os::getenv (by default it
crashes if not set, or if passing 'expect=false' then it returns an empty
string, but this means you can't differentiate between being unset and set to
empty string!) They should all be going through {{os::getenv}} once it returns
an Optionstring.
Add an Optionstring os::getenv() to stout
---
Key: MESOS-994
URL: https://issues.apache.org/jira/browse/MESOS-994
Project: Mesos
Issue Type: Improvement
Components: stout, technical debt
Reporter: Ian Downes
Assignee: Greg Mann
Labels: newbie
This would replace the common pattern of:
Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None()
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[
https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14529805#comment-14529805
]
Greg Mann commented on MESOS-994:
-
While working on this, I've discovered that in a couple places
(/3rdparty/libprocess/src/process.cpp for example), the stock getenv() is
called rather than os::getenv(), and in other cases
(/src/examples/low_level_scheduler_libprocess.cpp) they are both called:
{code}
if (getenv(MESOS_BUILD_DIR)) {
uri = path::join(os::getenv(MESOS_BUILD_DIR), src, test-executor);
}
{code}
What is the rationale for using these two different calls, or should
os::getenv() be used exclusively?
Add an Optionstring os::getenv() to stout
---
Key: MESOS-994
URL: https://issues.apache.org/jira/browse/MESOS-994
Project: Mesos
Issue Type: Improvement
Components: stout, technical debt
Reporter: Ian Downes
Assignee: Greg Mann
Labels: newbie
This would replace the common pattern of:
Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None()
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[ https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14518349#comment-14518349 ] Greg Mann commented on MESOS-994: - Ah OK, I see. That makes sense. I'll work on updating the second patch. Add an Optionstring os::getenv() to stout --- Key: MESOS-994 URL: https://issues.apache.org/jira/browse/MESOS-994 Project: Mesos Issue Type: Improvement Components: stout, technical debt Reporter: Ian Downes Assignee: Greg Mann Labels: newbie This would replace the common pattern of: Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None() -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-994) Add an Optionstring os::getenv() to stout
[
https://issues.apache.org/jira/browse/MESOS-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14517637#comment-14517637
]
Ian Downes commented on MESOS-994:
--
The intent here was to refactor the existing {{string os::getenv(string,
bool)}} to {{Optionstring os::getenv(string)}}, i.e., only have a single
version.
Ben's first patch above is good and the 2nd just needs updating.
Add an Optionstring os::getenv() to stout
---
Key: MESOS-994
URL: https://issues.apache.org/jira/browse/MESOS-994
Project: Mesos
Issue Type: Improvement
Components: stout, technical debt
Reporter: Ian Downes
Assignee: Greg Mann
Labels: newbie
This would replace the common pattern of:
Optionstring = os::hasenv() ? Optionstring(os::getenv()) : None()
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
