You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2019/02/17 17:48:39 UTC

Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

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

Review request for mesos, Till Toenshoff and Vinod Kone.


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


Repository: mesos


Description
-------

Previously `support/verify-reviews.py` would abort prematurely
whenever a review a patch could not be applied. This was due to
the `shell` function used to call `support/apply-reviews.py` invoking
`exit(1)` on the first error which stopped iteration over all
outstanding reviews.

In this patch that explicit call to `exit` is removed, and instead we
let a possible `subprocess.CalledProcessError` propagate up for it to
be handled at a higher level. Currently this will post a note on the
review in question to (1) notify the submitter, and (2) prevent the
review from being checked again.

With the changes here the script will not stop verify reviews in such
cases.


Diffs
-----

  support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 


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


Testing
-------

Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).


Thanks,

Benjamin Bannier


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69998', '69999', '70000']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2898/mesos-review-70000

Relevant logs:

- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2898/mesos-review-70000/logs/mesos-tests.log):

```
I0219 11:55:51.110582 51640 master.cpp:11392] Removing task fee746c8-e294-4310-b486-7bbaf95ccc6d with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-0000 on agent d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-S0 at slave(491)@192.10.1.6:63735 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0219 11:55:51.113581 51640 master.cpp:1269] Agent d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-S0 at slave(491)@192.10.1.6:63735 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0219 11:55:51.113581 51640 master.cpp:3292] Disconnecting agent d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-S0 at slave(491)@192.10.1.6:63735 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0219 11:55:51.113581 51640 master.cpp:3311] Deactivating agent d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-S0 at slave(491)@192.10.1.6:63735 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0219 11:55:51.114583 58152 hierarchical.cpp:390] Removed framework d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-0000
I0219 11:55:51.114583 58152 hierarchical.cpp:827] Agent d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-S0 deactivated
I0219 11:55:51.114583 57168 containerizer.cpp:2526] Destroying container 37e292c9-656d-4f8c-a681-d504e486cd4f in RUNNING state
I0219 11:55:51.115571 57168 containerizer.cpp:3193] Transitioning the state of container 37e292c9-656d-4f8c-a681-d504e486cd4f from RUNNING to DESTROYING
I0219 11:55:51.115571 57168 launcher.cpp:161] Asked to destroy container 37e292c9-656d-4f8c-a681-d504e486cd4f
W0219 11:55:51.116576 51604 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=4764 to p[       OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (683 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (699 ms total)

[----------] Global test environment tear-down
[==========] 1112 tests from 105 test cases ran. (516762 ms total)
[  PASSED  ] 1111 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 1 FAILED TEST
  YOU HAVE 232 DISABLED TESTS

eer '192.10.1.6:49292': IO failed with error code: The specified network name is no longer available.

W0219 11:55:51.117583 51604 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=2680 to peer '192.10.1.6:49293': IO failed with error code: The specified network name is no longer available.

I0219 11:55:51.178185 51880 containerizer.cpp:3032] Container 37e292c9-656d-4f8c-a681-d504e486cd4f has exited
I0219 11:55:51.208086 57812 master.cpp:1109] Master terminating
I0219 11:55:51.209095 55904 hierarchical.cpp:678] Removed agent d54a6fb7-2d5d-4a28-bea3-2a475ba8e3e8-S0
I0219 11:55:51.884948 51604 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Feb. 19, 2019, 10:43 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 10:43 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/2/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/#review212914
-----------------------------------------------------------


Fix it, then Ship it!





support/verify-reviews.py
Lines 182-186 (patched)
<https://reviews.apache.org/r/70000/#comment298791>

    We could now reference https://issues.apache.org/jira/browse/MESOS-9583 :)


- Till Toenshoff


On Feb. 19, 2019, 10:43 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 10:43 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/2/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On Feb. 19, 2019, 5:47 p.m., Vinod Kone wrote:
> > support/verify-reviews.py
> > Line 182 (original), 188 (patched)
> > <https://reviews.apache.org/r/70000/diff/3/?file=2125836#file2125836line193>
> >
> >     hmm. i don't think this is the right place to catch this exception. what if we don't get into this for loop at all if it's a single review chain?
> >     
> >     i think it should be in #197.
> 
> Benjamin Bannier wrote:
>     Good point. It seems it _also_ needs to be in l.197, but we still need to handle errors on l.188 as a patch from the dependencies might not apply (that's what we see currently). In that case we unfortunately post the message on the wrong patch until MESOS-9583 is fixed.

Updated after offline clarification with Vinod.


- Benjamin


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


On Feb. 19, 2019, 6:10 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 6:10 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/5/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On Feb. 19, 2019, 5:47 p.m., Vinod Kone wrote:
> > support/verify-reviews.py
> > Line 182 (original), 188 (patched)
> > <https://reviews.apache.org/r/70000/diff/3/?file=2125836#file2125836line193>
> >
> >     hmm. i don't think this is the right place to catch this exception. what if we don't get into this for loop at all if it's a single review chain?
> >     
> >     i think it should be in #197.

Good point. It seems it _also_ needs to be in l.197, but we still need to handle errors on l.188 as a patch from the dependencies might not apply (that's what we see currently). In that case we unfortunately post the message on the wrong patch until MESOS-9583 is fixed.


- Benjamin


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


On Feb. 19, 2019, 5:53 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 5:53 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/4/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Vinod Kone <vi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/#review212917
-----------------------------------------------------------




support/verify-reviews.py
Line 182 (original), 188 (patched)
<https://reviews.apache.org/r/70000/#comment298793>

    hmm. i don't think this is the right place to catch this exception. what if we don't get into this for loop at all if it's a single review chain?
    
    i think it should be in #197.


- Vinod Kone


On Feb. 19, 2019, 4:44 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 4:44 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/3/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

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



PASS: Mesos patch 70000 was successfully built and tested.

Reviews applied: `['69998', '69999', '70000']`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2899/mesos-review-70000

- Mesos Reviewbot Windows


On Feb. 19, 2019, 5:23 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 5:23 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/6/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Vinod Kone <vi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/#review212920
-----------------------------------------------------------


Ship it!




Ship It!

- Vinod Kone


On Feb. 19, 2019, 5:23 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 5:23 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/6/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/
-----------------------------------------------------------

(Updated Feb. 19, 2019, 6:23 p.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


Changes
-------

Remove a comment as suggested offline by Vinod


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


Repository: mesos


Description
-------

Previously `support/verify-reviews.py` would abort prematurely
whenever a review a patch could not be applied. This was due to
the `shell` function used to call `support/apply-reviews.py` invoking
`exit(1)` on the first error which stopped iteration over all
outstanding reviews.

In this patch that explicit call to `exit` is removed, and instead we
let a possible `subprocess.CalledProcessError` propagate up for it to
be handled at a higher level. Currently this will post a note on the
review in question to (1) notify the submitter, and (2) prevent the
review from being checked again.

With the changes here the script will not stop verify reviews in such
cases.


Diffs (updated)
-----

  support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 


Diff: https://reviews.apache.org/r/70000/diff/6/

Changes: https://reviews.apache.org/r/70000/diff/5-6/


Testing
-------

Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).


Thanks,

Benjamin Bannier


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/
-----------------------------------------------------------

(Updated Feb. 19, 2019, 6:10 p.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


Changes
-------

Really fix issue raised by Vinod


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


Repository: mesos


Description
-------

Previously `support/verify-reviews.py` would abort prematurely
whenever a review a patch could not be applied. This was due to
the `shell` function used to call `support/apply-reviews.py` invoking
`exit(1)` on the first error which stopped iteration over all
outstanding reviews.

In this patch that explicit call to `exit` is removed, and instead we
let a possible `subprocess.CalledProcessError` propagate up for it to
be handled at a higher level. Currently this will post a note on the
review in question to (1) notify the submitter, and (2) prevent the
review from being checked again.

With the changes here the script will not stop verify reviews in such
cases.


Diffs (updated)
-----

  support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 


Diff: https://reviews.apache.org/r/70000/diff/5/

Changes: https://reviews.apache.org/r/70000/diff/4-5/


Testing
-------

Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).


Thanks,

Benjamin Bannier


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/
-----------------------------------------------------------

(Updated Feb. 19, 2019, 5:53 p.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


Changes
-------

Address issue raised by Vinod


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


Repository: mesos


Description
-------

Previously `support/verify-reviews.py` would abort prematurely
whenever a review a patch could not be applied. This was due to
the `shell` function used to call `support/apply-reviews.py` invoking
`exit(1)` on the first error which stopped iteration over all
outstanding reviews.

In this patch that explicit call to `exit` is removed, and instead we
let a possible `subprocess.CalledProcessError` propagate up for it to
be handled at a higher level. Currently this will post a note on the
review in question to (1) notify the submitter, and (2) prevent the
review from being checked again.

With the changes here the script will not stop verify reviews in such
cases.


Diffs (updated)
-----

  support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 


Diff: https://reviews.apache.org/r/70000/diff/4/

Changes: https://reviews.apache.org/r/70000/diff/3-4/


Testing
-------

Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).


Thanks,

Benjamin Bannier


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/
-----------------------------------------------------------

(Updated Feb. 19, 2019, 5:44 p.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


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


Repository: mesos


Description
-------

Previously `support/verify-reviews.py` would abort prematurely
whenever a review a patch could not be applied. This was due to
the `shell` function used to call `support/apply-reviews.py` invoking
`exit(1)` on the first error which stopped iteration over all
outstanding reviews.

In this patch that explicit call to `exit` is removed, and instead we
let a possible `subprocess.CalledProcessError` propagate up for it to
be handled at a higher level. Currently this will post a note on the
review in question to (1) notify the submitter, and (2) prevent the
review from being checked again.

With the changes here the script will not stop verify reviews in such
cases.


Diffs (updated)
-----

  support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 


Diff: https://reviews.apache.org/r/70000/diff/3/

Changes: https://reviews.apache.org/r/70000/diff/2-3/


Testing
-------

Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).


Thanks,

Benjamin Bannier


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70000/
-----------------------------------------------------------

(Updated Feb. 19, 2019, 11:43 a.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


Changes
-------

Translate error from not applying patch to `ReviewError`


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


Repository: mesos


Description
-------

Previously `support/verify-reviews.py` would abort prematurely
whenever a review a patch could not be applied. This was due to
the `shell` function used to call `support/apply-reviews.py` invoking
`exit(1)` on the first error which stopped iteration over all
outstanding reviews.

In this patch that explicit call to `exit` is removed, and instead we
let a possible `subprocess.CalledProcessError` propagate up for it to
be handled at a higher level. Currently this will post a note on the
review in question to (1) notify the submitter, and (2) prevent the
review from being checked again.

With the changes here the script will not stop verify reviews in such
cases.


Diffs (updated)
-----

  support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 


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

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


Testing
-------

Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).


Thanks,

Benjamin Bannier


Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69998', '69999', '70000']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2895/mesos-review-70000

Relevant logs:

- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2895/mesos-review-70000/logs/mesos-tests.log):

```
W0217 18:55:33.626559 56944 slave.cpp:3928] Ignoring shutdown framework 43b9b291-43af-44b5-bc2a-ebe409702598-0000 because it is terminating
I0217 18:55:33.628559 44756 hierarchical.cpp:390] Removed framework 43b9b291-43af-44b5-bc2a-ebe409702598-0000
I0217 18:55:33.628559 56380 master.cpp:1269] Agent 43b9b291-43af-44b5-bc2a-ebe409702598-S0 at slave(491)@192.10.1.6:55485 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0217 18:55:33.629566 56380 master.cpp:3292] Disconnecting agent 43b9b291-43af-44b5-bc2a-ebe409702598-S0 at slave(491)@192.10.1.6:55485 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0217 18:55:33.629566 56380 master.cpp:3311] Deactivating agent 43b9b291-43af-44b5-bc2a-ebe409702598-S0 at slave(491)@192.10.1.6:55485 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0217 18:55:33.629566 49104 hierarchical.cpp:827] Agent 43b9b291-43af-44b5-bc2a-ebe409702598-S0 deactivated
I0217 18:55:33.630580 49104 containerizer.cpp:2526] Destroying container 7c684b00-4f8b-491b-bdd7-8ca85104cbe5 in RUNNING state
I0217 18:55:33.630580 49104 containerizer.cpp:3193] Transitioning the state of container 7c684b00-4f8b-491b-bdd7-8ca85104cbe5 from RUNNING to DESTROYING
I0217 18:55:33.631814 49104 launcher.cpp:161] Asked to destroy container 7c684b00-4f8b-491b-bdd7-8ca85104cbe5
W0217 18:55:33.632580 56768 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=2112 to peer '192.10.1.6:57429': IO failed with error code: The specified network name is no longer available.

W0217 18:55:33.632580 56768 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=2176 to peer '192.10.1.6:57428': IO failed w[       OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (686 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (705 ms total)

[----------] Global test environment tear-down
[==========] 1112 tests from 105 test cases ran. (517335 ms total)
[  PASSED  ] 1111 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 232 DISABLED TESTS

ith error code: The specified network name is no longer available.

I0217 18:55:33.715597 56380 containerizer.cpp:3032] Container 7c684b00-4f8b-491b-bdd7-8ca85104cbe5 has exited
I0217 18:55:33.745697 56944 master.cpp:1109] Master terminating
I0217 18:55:33.746562 52864 hierarchical.cpp:678] Removed agent 43b9b291-43af-44b5-bc2a-ebe409702598-S0
I0217 18:55:34.423573 56768 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Feb. 17, 2019, 9:48 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70000/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2019, 9:48 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
>     https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/70000/diff/1/
> 
> 
> Testing
> -------
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. The script attempted to post a review on the last patch in the chain instead of aborting (see the `TODO` in the code on why we weren't able to diagnose the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>