You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2013/02/14 02:43:00 UTC
Re: Review Request: Slave Restart (Part 6): Added os::islink and substituted
stat() with lstat() in os.hpp
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8681/#review16551
-----------------------------------------------------------
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/8681/#comment35122>
What's the justification for changing this one? I don't really have objections, but I'd like to understand the thought behind this
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/8681/#comment35123>
Here I'm not sure what to do either.. I feel like the caller would need to know the semantics they want in terms of links.
- Ben Mahler
On Jan. 28, 2013, 4:29 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8681/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 4:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Needed this for my next review.
>
>
> Diffs
> -----
>
> third_party/libprocess/include/stout/os.hpp fdba9cadc38251b192e9d395a0a1ab1f08327596
>
> Diff: https://reviews.apache.org/r/8681/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Slave Restart (Part 6): Added os::islink and substituted
stat() with lstat() in os.hpp
Posted by Vinod Kone <vi...@gmail.com>.
> On Feb. 14, 2013, 1:43 a.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/os.hpp, line 401
> > <https://reviews.apache.org/r/8681/diff/3/?file=252338#file252338line401>
> >
> > What's the justification for changing this one? I don't really have objections, but I'd like to understand the thought behind this
I think its intuitive when you ask "if a link exists", to answer with whether that link exists, not whether the target of the link exists.
> On Feb. 14, 2013, 1:43 a.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/os.hpp, line 413
> > <https://reviews.apache.org/r/8681/diff/3/?file=252338#file252338line413>
> >
> > Here I'm not sure what to do either.. I feel like the caller would need to know the semantics they want in terms of links.
See my answer to Ben's question above.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8681/#review16551
-----------------------------------------------------------
On Jan. 28, 2013, 4:29 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8681/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 4:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Needed this for my next review.
>
>
> Diffs
> -----
>
> third_party/libprocess/include/stout/os.hpp fdba9cadc38251b192e9d395a0a1ab1f08327596
>
> Diff: https://reviews.apache.org/r/8681/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Slave Restart (Part 6): Added os::islink and substituted
stat() with lstat() in os.hpp
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 14, 2013, 1:43 a.m., Ben Mahler wrote:
> >
Can you dig up other libraries as examples?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8681/#review16551
-----------------------------------------------------------
On Jan. 28, 2013, 4:29 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8681/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 4:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Needed this for my next review.
>
>
> Diffs
> -----
>
> third_party/libprocess/include/stout/os.hpp fdba9cadc38251b192e9d395a0a1ab1f08327596
>
> Diff: https://reviews.apache.org/r/8681/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Slave Restart (Part 6): Added os::islink and substituted
stat() with lstat() in os.hpp
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 14, 2013, 1:43 a.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/os.hpp, line 413
> > <https://reviews.apache.org/r/8681/diff/3/?file=252338#file252338line413>
> >
> > Here I'm not sure what to do either.. I feel like the caller would need to know the semantics they want in terms of links.
>
> Vinod Kone wrote:
> See my answer to Ben's question above.
Ditto here. I think lstat is the right default, but I can see mistakes being made.
> On Feb. 14, 2013, 1:43 a.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/os.hpp, line 401
> > <https://reviews.apache.org/r/8681/diff/3/?file=252338#file252338line401>
> >
> > What's the justification for changing this one? I don't really have objections, but I'd like to understand the thought behind this
>
> Vinod Kone wrote:
> I think its intuitive when you ask "if a link exists", to answer with whether that link exists, not whether the target of the link exists.
Fair enough, I think lstat is better than stat, but I could still see mistakes being made. So I don't like forcing it either way.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8681/#review16551
-----------------------------------------------------------
On Jan. 28, 2013, 4:29 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8681/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 4:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Needed this for my next review.
>
>
> Diffs
> -----
>
> third_party/libprocess/include/stout/os.hpp fdba9cadc38251b192e9d395a0a1ab1f08327596
>
> Diff: https://reviews.apache.org/r/8681/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>