mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 58928: Update process tests to use a non-zero UPID.
Date Fri, 12 May 2017 00:38:55 GMT

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


Fix it, then Ship it!




Test changes look good, just noticed that this is a few independent patches, could you pull
them into separate smaller patches?

(1) Improve the comments for server socket IP selection in libprocess initialization.
(2) Fix a logging message in libprocess initialization. (should be using strerror, not hstrerror)
(3) Add a process ID for RemoteProcess in the libprocess tests. (to make it easier to debug)
(4) Update the libprocess tests to use a non-zero UPID.


3rdparty/libprocess/src/process.cpp
Lines 1156-1158 (original), 1156-1158 (patched)
<https://reviews.apache.org/r/58928/#comment247971>

    Would you mind pulling out the comment change here and below ("taking the first result")
into a separate single patch that is described as clarifying the hostname lookup comments
in libprocess intiialization? Will be able to get this committed for you quickly.



3rdparty/libprocess/src/process.cpp
Line 1164 (original), 1164 (patched)
<https://reviews.apache.org/r/58928/#comment247970>

    Ah thanks for fixing this, this can just be:
    
    ```
    PLOG(FATAL) << "Failed to initialize, gethostname";
    ```
    
    Also, since its an independent change, can you make this its own patch about fixing the
logging message here? I can get it committed for you quickly.



3rdparty/libprocess/src/tests/process_tests.cpp
Line 1295 (original), 1295 (patched)
<https://reviews.apache.org/r/58928/#comment247972>

    This seems like an unrelated change? Do you want to pull it out if so?
    
    Small independent patches are faster to review and commit :)



3rdparty/libprocess/src/tests/test_linkee.cpp
Lines 141 (patched)
<https://reviews.apache.org/r/58928/#comment247973>

    ```
    PLOG(FATAL) << "Failed to get the local hostname";
    ```


- Benjamin Mahler


On May 10, 2017, 6:05 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58928/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 6:05 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
>     https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Some of the remote process tests were using a UPID with a zero
> IP address field. In order to enable subsequent IP address
> validation in the libprocess receiver, update these tests to
> use a UPID containing the real IP address of the sender.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
>   3rdparty/libprocess/src/tests/process_tests.cpp bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115

>   3rdparty/libprocess/src/tests/test_linkee.cpp 921d67695bc0e4d601e9f74fbc625d69bf36ba50

> 
> 
> Diff: https://reviews.apache.org/r/58928/diff/3/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message