You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andy Pang <pa...@huawei.com> on 2017/04/10 07:17:32 UTC
Review Request 58287: Print corresponding address when socket
shutdown.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos
Description
-------
Print corresponding address when socket shutdown.
Default just print socket 'fd',it's not convenient
to find corresponding address.
Diffs
-----
3rdparty/libprocess/src/process.cpp d0cba0c
Diff: https://reviews.apache.org/r/58287/diff/1/
Testing
-------
make check
Thanks,
Andy Pang
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review171428
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58287]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 10, 2017, 7:17 a.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 10, 2017, 7:17 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp d0cba0c
>
>
> Diff: https://reviews.apache.org/r/58287/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review171615
-----------------------------------------------------------
3rdparty/libprocess/src/process.cpp
Lines 2362-2363 (patched)
<https://reviews.apache.org/r/58287/#comment244566>
can we do
```
<< "Failed to shutdown socket with fd " << socket.get()
<< ", address " << stringify(socket.address.get())
<< ": " << shutdown.error();
```
- Vinod Kone
On April 10, 2017, 7:17 a.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 10, 2017, 7:17 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp d0cba0c
>
>
> Diff: https://reviews.apache.org/r/58287/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review171879
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58287]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 13, 2017, 12:44 p.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 12:44 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> File Attachments
> ----------------
>
> process.patch
> https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review172017
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On April 13, 2017, 12:44 p.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 12:44 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> File Attachments
> ----------------
>
> process.patch
> https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review173114
-----------------------------------------------------------
Ship it!
Made some minor adjustments before committing.
- Vinod Kone
On April 20, 2017, 11:34 a.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 11:34 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> File Attachments
> ----------------
>
> process.patch
> https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review172495
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58287]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 20, 2017, 7:34 a.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 7:34 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> File Attachments
> ----------------
>
> process.patch
> https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Andy Pang <pa...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/
-----------------------------------------------------------
(Updated \u56db\u6708 20, 2017, 11:34 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos
Description
-------
Print corresponding address when socket shutdown.
Default just print socket 'fd',it's not convenient
to find corresponding address.
Diffs (updated)
-----
3rdparty/libprocess/src/process.cpp 92efa91
Diff: https://reviews.apache.org/r/58287/diff/5/
Changes: https://reviews.apache.org/r/58287/diff/4-5/
Testing
-------
make check
File Attachments
----------------
process.patch
https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
Thanks,
Andy Pang
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Andy Pang <pa...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/
-----------------------------------------------------------
(Updated \u56db\u6708 20, 2017, 9:09 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos
Description
-------
Print corresponding address when socket shutdown.
Default just print socket 'fd',it's not convenient
to find corresponding address.
Diffs (updated)
-----
3rdparty/libprocess/src/process.cpp 92efa91
Diff: https://reviews.apache.org/r/58287/diff/4/
Changes: https://reviews.apache.org/r/58287/diff/3-4/
Testing
-------
make check
File Attachments
----------------
process.patch
https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
Thanks,
Andy Pang
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review172019
-----------------------------------------------------------
3rdparty/libprocess/src/process.cpp
Line 2361 (original), 2361-2362 (patched)
<https://reviews.apache.org/r/58287/#comment245081>
Looks like these lines are greater than 80 chars long. Can you wrap them please?
```
? mesos git:(master) ? ./support/apply-reviews.py -c -r 58287
2017-04-14 12:23:29 URL:https://reviews.apache.org/r/58287/diff/raw/ [1178/1178] -> "58287.patch" [1]
Checking 1 C++ file
3rdparty/libprocess/src/process.cpp:2361: Lines should be <= 80 characters long [whitespace/line_length] [2]
3rdparty/libprocess/src/process.cpp:2362: Lines should be <= 80 characters long [whitespace/line_length] [2]
3rdparty/libprocess/src/process.cpp:2445: Lines should be <= 80 characters long [whitespace/line_length] [2]
3rdparty/libprocess/src/process.cpp:2446: Lines should be <= 80 characters long [whitespace/line_length] [2]
Total errors found: 1
No Python files to lint
```
- Vinod Kone
On April 13, 2017, 12:44 p.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 12:44 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> File Attachments
> ----------------
>
> process.patch
> https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Andy Pang <pa...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/
-----------------------------------------------------------
(Updated \u56db\u6708 13, 2017, 12:44 p.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos
Description
-------
Print corresponding address when socket shutdown.
Default just print socket 'fd',it's not convenient
to find corresponding address.
Diffs (updated)
-----
3rdparty/libprocess/src/process.cpp 92efa91
Diff: https://reviews.apache.org/r/58287/diff/3/
Changes: https://reviews.apache.org/r/58287/diff/2-3/
Testing
-------
make check
File Attachments (updated)
----------------
process.patch
https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
Thanks,
Andy Pang
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Vinod Kone <vi...@gmail.com>.
> On April 12, 2017, 6:39 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/process.cpp
> > Lines 2362 (patched)
> > <https://reviews.apache.org/r/58287/diff/2/?file=1689528#file1689528line2362>
> >
> > What if socket.address returns an Error? Let's not print the address if it is an error:
> >
> > ```
> > Try<network::Address> socketAddress = socket.address();
> >
> > LOG(ERROR) << "Failed to shutdown socket with fd " << socket.get() << ", address "
> > << (socketAddress.isSome() ? stringify(socketAddress.get()) : "N/A")
> > << ": " << shutdown.error();
> > ```
Marking this issue and the below one as "fixed". Andy, for future reference once you fix an issue please mark it as fixed.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review171772
-----------------------------------------------------------
On April 13, 2017, 12:44 p.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 12:44 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> File Attachments
> ----------------
>
> process.patch
> https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review171772
-----------------------------------------------------------
3rdparty/libprocess/src/process.cpp
Lines 2362 (patched)
<https://reviews.apache.org/r/58287/#comment244776>
What if socket.address returns an Error? Let's not print the address if it is an error:
```
Try<network::Address> socketAddress = socket.address();
LOG(ERROR) << "Failed to shutdown socket with fd " << socket.get() << ", address "
<< (socketAddress.isSome() ? stringify(socketAddress.get()) : "N/A")
<< ": " << shutdown.error();
```
3rdparty/libprocess/src/process.cpp
Lines 2446 (patched)
<https://reviews.apache.org/r/58287/#comment244777>
Ditto.
- Jie Yu
On April 12, 2017, 1:42 a.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 1:42 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/#review171703
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58287]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 12, 2017, 1:42 a.m., Andy Pang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 1:42 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 92efa91
>
>
> Diff: https://reviews.apache.org/r/58287/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Andy Pang
>
>
Re: Review Request 58287: Print corresponding address when socket
shutdown.
Posted by Andy Pang <pa...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58287/
-----------------------------------------------------------
(Updated \u56db\u6708 12, 2017, 1:42 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Fixed as suggestion.
Repository: mesos
Description
-------
Print corresponding address when socket shutdown.
Default just print socket 'fd',it's not convenient
to find corresponding address.
Diffs (updated)
-----
3rdparty/libprocess/src/process.cpp 92efa91
Diff: https://reviews.apache.org/r/58287/diff/2/
Changes: https://reviews.apache.org/r/58287/diff/1-2/
Testing
-------
make check
Thanks,
Andy Pang