You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Akash Gupta <ak...@hotmail.com> on 2018/05/04 17:27:04 UTC

Review Request 66962: Windows: Added tests for async IO functions.

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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.


Bugs: MESOS-8674
    https://issues.apache.org/jira/browse/MESOS-8674


Repository: mesos


Description
-------

New asynchronous functions were introduced to the Windows stout read
and write implementations, so eztra tests were added.


Diffs
-----

  3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 


Diff: https://reviews.apache.org/r/66962/diff/1/


Testing
-------


Thanks,

Akash Gupta

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review202467
-----------------------------------------------------------



PASS: Mesos patch 66962 was successfully built and tested.

Reviews applied: `['66952', '66953', '66954', '66955', '66956', '66957', '66958', '66959', '66960', '66961', '66962']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66962

- Mesos Reviewbot Windows


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Akash Gupta <ak...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review202469
-----------------------------------------------------------



By the way, this patch set is roughly half of the changes for the IOCP backend. These focus mainly on the stout changes. I'm currently cleaning up some libprocess code so that I can post the remaining half.

- Akash Gupta


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Radhika Jandhyala via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review202554
-----------------------------------------------------------




3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 793 (patched)
<https://reviews.apache.org/r/66962/#comment284370>

    Since read_async returned result before write_async finished, is result being updated after it is returnend at line 779?


- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review202593
-----------------------------------------------------------


Fix it, then Ship it!





3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 663 (patched)
<https://reviews.apache.org/r/66962/#comment284475>

    `s/os::getcwd()/sandbox`



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 674-675 (patched)
<https://reviews.apache.org/r/66962/#comment284476>

    `s/ASSERT/EXPECT`



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 686-687 (patched)
<https://reviews.apache.org/r/66962/#comment284477>

    Ditto.



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 707-710 (patched)
<https://reviews.apache.org/r/66962/#comment284478>

    Ditto, and the other non-fatal ones.



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 726-729 (patched)
<https://reviews.apache.org/r/66962/#comment284481>

    Nit: I can't remember indentation rules, did you run clang-format?



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 754-755 (patched)
<https://reviews.apache.org/r/66962/#comment284480>

    EXPECTs.


- Andrew Schwartzmeyer


On May 4, 2018, 10:27 a.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 10:27 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Radhika Jandhyala via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review202510
-----------------------------------------------------------


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Radhika Jandhyala via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review202479
-----------------------------------------------------------




3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 724-729 (patched)
<https://reviews.apache.org/r/66962/#comment284299>

    I believe definitions for read_async/write_async might not be in master as yet. Would this break the build?



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 769 (patched)
<https://reviews.apache.org/r/66962/#comment284300>

    Does this really block? Then execution would not proceed to read_async?


- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review203630
-----------------------------------------------------------



PASS: Mesos patch 66962 was successfully built and tested.

Reviews applied: `['66952', '66954', '66955', '66956', '66957', '66958', '66959', '66960', '66961', '66962']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66962

- Mesos Reviewbot Windows


On May 22, 2018, 10:55 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 22, 2018, 10:55 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/2/
> 
> 
> Testing
> -------
> 
> Ran stout/libprocess/mesos-tests on Windows. Ran make check on Linux.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review203698
-----------------------------------------------------------


Ship it!




Ship It!

- Andrew Schwartzmeyer


On May 22, 2018, 3:55 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 22, 2018, 3:55 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so extra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/2/
> 
> 
> Testing
> -------
> 
> Ran stout/libprocess/mesos-tests on Windows. Ran make check on Linux.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/#review203650
-----------------------------------------------------------



Patch looks great!

Reviews applied: [67248, 66952, 66954, 66955, 66956, 66957, 66958, 66959, 66960, 66961, 66962]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 22, 2018, 10:55 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> -----------------------------------------------------------
> 
> (Updated May 22, 2018, 10:55 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
>     https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/2/
> 
> 
> Testing
> -------
> 
> Ran stout/libprocess/mesos-tests on Windows. Ran make check on Linux.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

Posted by Akash Gupta <ak...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66962/
-----------------------------------------------------------

(Updated May 22, 2018, 10:55 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.


Changes
-------

Addressed feedback.


Bugs: MESOS-8670
    https://issues.apache.org/jira/browse/MESOS-8670


Repository: mesos


Description
-------

New asynchronous functions were introduced to the Windows stout read
and write implementations, so eztra tests were added.


Diffs (updated)
-----

  3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 


Diff: https://reviews.apache.org/r/66962/diff/2/

Changes: https://reviews.apache.org/r/66962/diff/1-2/


Testing
-------


Thanks,

Akash Gupta