You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Eric Chung <ci...@gmail.com> on 2017/09/21 04:23:47 UTC

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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

(Updated Sept. 21, 2017, 4:23 a.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
-------

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.


Diffs (updated)
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 


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

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


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



FAIL: Some Mesos tests failed.

Reviews applied: `['61172']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose --gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`

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

Relevant logs:

- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61172/logs/mesos-tests-stdout.log):

```
[ RUN      ] ContentType/SchedulerTest.SchedulerReconnect/0
[       OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (249 ms)
[ RUN      ] ContentType/SchedulerTest.SchedulerReconnect/1
[       OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (253 ms)
[----------] 30 tests from ContentType/SchedulerTest (26578 ms total)

[----------] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN      ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[       OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (902 ms)
[ RUN      ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[       OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 (1023 ms)
[----------] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2053 ms total)

[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN      ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[       OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (133 ms)
[ RUN      ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[       OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (151 ms)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (370 ms total)

[----------] Global test environment tear-down
[==========] 634 tests from 67 test cases ran. (358685 ms total)
[  PASSED  ] 633 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.TaskWithFileURI/0, where GetParam() = "mesos"

 1 FAILED TEST
  YOU HAVE 182 DISABLED TESTS

```

- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61172/logs/mesos-tests-stderr.log):

```
I0930 00:17:39.407632 31184 master.cpp:8438] Removing framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 (default)
I0930 00:17:39.407632 31184 master.cpp:3317] Deactivating framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 (default)
I0930 00:17:39.408632 30504 hierarchical.cpp:412] Deactivated framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000
I0930 00:17:39.408632 28900 slave.cpp:3239] Shutting down framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000
I0930 00:17:39.408632 31184 master.cpp:9136] Updating the state of task 5213cf5b-e0d5-4579-bdcf-84cb469c141b of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I0930 00:17:39.408632 28900 slave.cpp:5746] Shutting down executor 'default' of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 (via HTTP)
I0930 00:17:39.412633 31184 master.cpp:9230] Removing task 5213cf5b-e0d5-4579-bdcf-84cb469c141b with resources [{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}] of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 on agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0930 00:17:39.426633 31184 master.cpp:9259] Removing executor 'default' with resources [] of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 on agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
E0930 00:17:39.444922 31364 scheduler.cpp:649] End-Of-File received from master. The master closed the event stream
I0930 00:17:39.443634 28900 hierarchical.cpp:355] Removed framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000
I0930 00:17:39.445634 31248 scheduler.cpp:444] Re-detecting master
I0930 00:17:39.452633 31248 scheduler.cpp:470] New master detected at master@10.3.1.7:55700
I0930 00:17:39.468634 30652 slave.cpp:5411] Executor 'default' of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 exited with status 0
I0930 00:17:39.469635 30652 slave.cpp:5515] Cleaning up executor 'default' of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 (via HTTP)
W0930 00:17:39.469635 28900 master.cpp:7089] Ignoring unknown exited executor 'default' of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 on agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0930 00:17:39.470635 31248 gc.cpp:90] Scheduling 'C:\Users\mesos\AppData\Local\Temp\2\v3junw\slaves\29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0\frameworks\29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000\executors\default\runs\9f970063-d337-47e6-93de-078b9f9794ce' for gc 6.9999945528237days in the future
I0930 00:17:39.470635 30652 slave.cpp:5622] Cleaning up framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000
I0930 00:17:39.482637 30504 status_update_manager.cpp:285] Closing status update streams for framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000
I0930 00:17:39.482637 31248 gc.cpp:90] Scheduling 'C:\Users\mesos\AppData\Local\Temp\2\v3junw\slaves\29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0\frameworks\29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000\executors\default' for gc 6.9999945528237days in the future
I0930 00:17:39.483635 31248 gc.cpp:90] Scheduling 'C:\Users\mesos\AppData\Local\Temp\2\v3junw\slaves\29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0\frameworks\29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000' for gc 6.99999440236148days in the future
I0930 00:17:39.483635 30652 slave.cpp:869] Agent terminating
I0930 00:17:39.496634 31364 master.cpp:1321] Agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0930 00:17:39.496634 31364 master.cpp:3354] Disconnecting agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0930 00:17:39.496634 31364 master.cpp:3373] Deactivating agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0930 00:17:39.497635 28900 hierarchical.cpp:690] Agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 deactivated
I0930 00:17:39.525638 31392 master.cpp:1163] Master terminating
I0930 00:17:39.528636 31364 hierarchical.cpp:626] Removed agent 29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0
W0930 00:17:39.531636 31392 master.hpp:2882] Failed to close HTTP pipe for 29bea81c-7cc9-4cae-8b13-4e61edd45a51-0000 (default)
I0930 00:17:40.393668 27748 process.cpp:1068] Failed to accept socket: future discarded
```

- Mesos Reviewbot Windows


On Sept. 29, 2017, 9:25 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2017, 9:25 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   src/python/pylint.config 9f0361d160fa82038ccefcbc146e52f0af8f99e8 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/6/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review193850
-----------------------------------------------------------




src/python/lib/mesos/exceptions.py
Lines 56 (patched)
<https://reviews.apache.org/r/61172/#comment272478>

    consider adding class attribute indicating status code corresponding to this exception



src/python/lib/mesos/http.py
Lines 45 (patched)
<https://reviews.apache.org/r/61172/#comment272479>

    break into multiple lines



src/python/lib/mesos/http.py
Lines 54 (patched)
<https://reviews.apache.org/r/61172/#comment272480>

    change to boolean



src/python/lib/mesos/http.py
Lines 105 (patched)
<https://reviews.apache.org/r/61172/#comment272481>

    update docstring



src/python/lib/mesos/http.py
Lines 114-116 (patched)
<https://reviews.apache.org/r/61172/#comment272486>

    unnecessary -- just store a copy of the dictionary



src/python/lib/mesos/http.py
Lines 118 (patched)
<https://reviews.apache.org/r/61172/#comment272482>

    rename to default_headers



src/python/lib/mesos/http.py
Lines 135 (patched)
<https://reviews.apache.org/r/61172/#comment272483>

    `return Resource(...)`



src/python/lib/mesos/http.py
Lines 137 (patched)
<https://reviews.apache.org/r/61172/#comment272487>

    just use the variable, no need for extra method for headers



src/python/lib/mesos/http.py
Lines 149 (patched)
<https://reviews.apache.org/r/61172/#comment272488>

    boolean



src/python/lib/mesos/http.py
Lines 174 (patched)
<https://reviews.apache.org/r/61172/#comment272491>

    use variable



src/python/lib/mesos/http.py
Lines 187-188 (patched)
<https://reviews.apache.org/r/61172/#comment272492>

    move to top, initialize headers with REQUEST_GZIP_HEADERS if headers is None



src/python/lib/mesos/http.py
Lines 191-196 (patched)
<https://reviews.apache.org/r/61172/#comment272493>

    keep order consistent



src/python/lib/mesos/http.py
Lines 199 (patched)
<https://reviews.apache.org/r/61172/#comment272495>

    add comment about why try..except isn't used here



src/python/lib/mesos/http.py
Lines 200 (patched)
<https://reviews.apache.org/r/61172/#comment272494>

    known_exception_class



src/python/lib/mesos/http.py
Lines 200-204 (patched)
<https://reviews.apache.org/r/61172/#comment272496>

    do something like `if response.status_code in self.ERROR_CODE_MAP`



src/python/lib/mesos/http.py
Lines 205 (patched)
<https://reviews.apache.org/r/61172/#comment272497>

    line break before return



src/python/lib/mesos/http.py
Lines 212 (patched)
<https://reviews.apache.org/r/61172/#comment272498>

    boolean



src/python/lib/mesos/http.py
Lines 213 (patched)
<https://reviews.apache.org/r/61172/#comment272499>

    after params



src/python/lib/mesos/http.py
Lines 219-239 (patched)
<https://reviews.apache.org/r/61172/#comment272500>

    update



src/python/lib/mesos/http.py
Lines 244 (patched)
<https://reviews.apache.org/r/61172/#comment272501>

    "We"



src/python/lib/mesos/http.py
Lines 269-279 (patched)
<https://reviews.apache.org/r/61172/#comment272503>

    reorder excepts and add comments



src/python/lib/mesos/http.py
Lines 270 (patched)
<https://reviews.apache.org/r/61172/#comment272502>

    optionally include the origianl exception; raise MesosException instead



src/python/lib/mesos/http.py
Lines 321 (patched)
<https://reviews.apache.org/r/61172/#comment272504>

    1. raise MesosException
    2. put space on next line
    3. nit: make lines equal length



src/python/lib/mesos/http.py
Lines 328 (patched)
<https://reviews.apache.org/r/61172/#comment272505>

    line break before return



src/python/lib/tests/test_http.py
Lines 36 (patched)
<https://reviews.apache.org/r/61172/#comment272506>

    update comment



src/python/lib/tests/test_http.py
Lines 37 (patched)
<https://reviews.apache.org/r/61172/#comment272507>

    explicit variable names



src/python/lib/tests/test_http.py
Lines 41 (patched)
<https://reviews.apache.org/r/61172/#comment272509>

    break params into list



src/python/lib/tests/test_http.py
Lines 42-58 (patched)
<https://reviews.apache.org/r/61172/#comment272508>

    reformat


- Eric Chung


On Nov. 30, 2017, 10:16 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2017, 10:16 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/7/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.

> On Jan. 18, 2018, 1:59 p.m., Kevin Klues wrote:
> > src/python/lib/mesos/exceptions.py
> > Lines 27-33 (patched)
> > <https://reviews.apache.org/r/61172/diff/7-9/?file=1904480#file1904480line27>
> >
> >     I would prefer to do this at each call site instead of wrapping it up and hiding it in here. This will keep it consistent with the rest of the code base.
> >     
> >     That said, I do like keeping track of the original exception. So something like the following would be better in my opinion:
> >     
> >     ```
> >         def __init__(self, message=None, original_exception=None):
> >             self.original_exception = original_exception
> >             super(MesosException, self).__init__(message)
> >     ```
> >     
> >     Is the name `original_exception` the best name here? Is there a more standard way of wrapping exceptions like this in python and keeping track of the original ones?

it seems there is no good solution for this use case in python 2, however it is supported natively in python 3 via exception chaining: https://www.python.org/dev/peps/pep-3134/ maybe another incentive for us to migrate. :)

just curious though, are you saying i shouldn't append the exception to the message?


- Eric


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


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/10/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review195718
-----------------------------------------------------------




src/python/lib/mesos/exceptions.py
Lines 27-33 (patched)
<https://reviews.apache.org/r/61172/#comment275006>

    I would prefer to do this at each call site instead of wrapping it up and hiding it in here. This will keep it consistent with the rest of the code base.
    
    That said, I do like keeping track of the original exception. So something like the following would be better in my opinion:
    
    ```
        def __init__(self, message=None, original_exception=None):
            self.original_exception = original_exception
            super(MesosException, self).__init__(message)
    ```
    
    Is the name `original_exception` the best name here? Is there a more standard way of wrapping exceptions like this in python and keeping track of the original ones?



src/python/lib/mesos/exceptions.py
Line 50 (original), 61 (patched)
<https://reviews.apache.org/r/61172/#comment275007>

    Can you add the text `The url '{url}' ...` to the beginning of the string so that it composes well with other exceptions. They should all begin with a capital letter.



src/python/lib/mesos/http.py
Lines 201-205 (original), 218-221 (patched)
<https://reviews.apache.org/r/61172/#comment275008>

    Can we simplify this to:
    ```
    if know_exception:
        raise known_exception(response)
    else:
        raise MesosHTTPException(response)
    ```



src/python/lib/mesos/http.py
Line 278 (original), 290 (patched)
<https://reviews.apache.org/r/61172/#comment275009>

    Error strings should beign with a capital letter.



src/python/lib/mesos/http.py
Line 322 (original), 333 (patched)
<https://reviews.apache.org/r/61172/#comment275010>

    Error strings should beign with a capital letter.



src/python/lib/tests/test_http.py
Line 283 (original), 286 (patched)
<https://reviews.apache.org/r/61172/#comment275011>

    The tmeout parameter should be moved to the next line


- Kevin Klues


On Jan. 18, 2018, 4:25 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2018, 4:25 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/9/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



PASS: Mesos patch 61172 was successfully built and tested.

Reviews applied: `['61172']`

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

- Mesos Reviewbot Windows


On Jan. 18, 2018, 4:25 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2018, 4:25 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/9/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review196638
-----------------------------------------------------------


Ship it!




Ship It!

- Armand Grillet


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/10/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/
-----------------------------------------------------------

(Updated Feb. 7, 2018, 3:13 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


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


Repository: mesos


Description
-------

Added mesos.http and mesos.exceptions for CLI.

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


Diffs
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/10/


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



PASS: Mesos patch 61172 was successfully built and tested.

Reviews applied: `['61172']`

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

- Mesos Reviewbot Windows


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/10/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review196992
-----------------------------------------------------------


Ship it!




Ship It!

- Kevin Klues


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/10/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/
-----------------------------------------------------------

(Updated Jan. 19, 2018, 8:48 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
-------

Added mesos.http and mesos.exceptions for CLI.

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


Diffs (updated)
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/10/

Changes: https://reviews.apache.org/r/61172/diff/9-10/


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/
-----------------------------------------------------------

(Updated Jan. 18, 2018, 4:25 a.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
-------

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


Diffs (updated)
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/9/

Changes: https://reviews.apache.org/r/61172/diff/8-9/


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review194604
-----------------------------------------------------------




src/python/lib/mesos/http.py
Lines 149 (patched)
<https://reviews.apache.org/r/61172/#comment273467>

    actually this has to be None if we want to allow defaults and overrides. a None passed in indicates that we're not overriding -- if the default value is boolean, there is no way to tell whether this was passed in by the user or whether it was just the default value.


- Eric Chung


On Jan. 1, 2018, 1:52 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 1, 2018, 1:52 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> addressed comments
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/8/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.

> On Jan. 5, 2018, 3:23 p.m., Armand Grillet wrote:
> > src/python/cli_new/bootstrap
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/61172/diff/8/?file=1929289#file1929289line72>
> >
> >     This file does not exist, is that normal?

it should...
```
~/mesos/src/python/lib$ git blame requirements-test.in
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 1) coverage
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 2) mock
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 3) pytest
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 4) pytest-cov
```


> On Jan. 5, 2018, 3:23 p.m., Armand Grillet wrote:
> > src/python/lib/mesos/http.py
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/61172/diff/8/?file=1929291#file1929291line72>
> >
> >     We should do that everywhere in the CLI, this is a useful addition. We could also use [mypy](http://www.mypy-lang.org/).

that looks pretty cool! we could also migrate to python3 which supports type annotations


- Eric


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


On Jan. 1, 2018, 1:52 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 1, 2018, 1:52 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> addressed comments
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/8/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review194862
-----------------------------------------------------------



Some small issues to fix, there are also some typos in the commit description.


src/python/cli_new/bootstrap
Lines 72 (patched)
<https://reviews.apache.org/r/61172/#comment273927>

    This file does not exist, is that normal?



src/python/lib/mesos/exceptions.py
Lines 26 (patched)
<https://reviews.apache.org/r/61172/#comment273928>

    s/mesos/Mesos



src/python/lib/mesos/exceptions.py
Lines 30 (patched)
<https://reviews.apache.org/r/61172/#comment273929>

    We use the syntax `"{msg}: {exception}".format(msg=message, exception=original_exception)` in the rest of our Python codebase. I do not have an opinion regarding the syntax but we should follow the current one in that review request and do a refactoring later if necessary.



src/python/lib/mesos/exceptions.py
Lines 60 (patched)
<https://reviews.apache.org/r/61172/#comment273930>

    s/'/"



src/python/lib/mesos/exceptions.py
Lines 75 (patched)
<https://reviews.apache.org/r/61172/#comment273931>

    



src/python/lib/mesos/exceptions.py
Lines 83 (patched)
<https://reviews.apache.org/r/61172/#comment273934>

    Unecessary comma.



src/python/lib/mesos/exceptions.py
Lines 106 (patched)
<https://reviews.apache.org/r/61172/#comment273935>

    "Given when " is unecessary.



src/python/lib/mesos/http.py
Lines 20 (patched)
<https://reviews.apache.org/r/61172/#comment273936>

    s/`restful API`/`RESTful API.`



src/python/lib/mesos/http.py
Lines 25 (patched)
<https://reviews.apache.org/r/61172/#comment273939>

    Let's do `from urlparse import urlparse` instead as we only use this once for that function in that file.



src/python/lib/mesos/http.py
Lines 33 (patched)
<https://reviews.apache.org/r/61172/#comment273937>

    Not in alphabetical order but it makes sense due to the inheritance of these classes.



src/python/lib/mesos/http.py
Lines 72 (patched)
<https://reviews.apache.org/r/61172/#comment273938>

    We should do that everywhere in the CLI, this is a useful addition. We could also use [mypy](http://www.mypy-lang.org/).



src/python/lib/mesos/http.py
Lines 117 (patched)
<https://reviews.apache.org/r/61172/#comment273944>

    Will just be `urlparse(url)` following previous comment.



src/python/lib/mesos/http.py
Lines 245 (patched)
<https://reviews.apache.org/r/61172/#comment273940>

    ```
    :param use_gzip_encoding: boolean indicating whether to pass gzip
                              encoding in the request headers or not
    ```



src/python/lib/mesos/http.py
Lines 261 (patched)
<https://reviews.apache.org/r/61172/#comment273941>

    I have some trouble reading this comment. I would write something like "We retry only when it makes sense: either due to a network partition (e.g. connection errors) or if the request failed due to a server error such as 500s, timeouts, and so on."



src/python/lib/tests/test_http.py
Lines 239 (patched)
<https://reviews.apache.org/r/61172/#comment273943>

    s/resrouce/resource


- Armand Grillet


On Jan. 1, 2018, 1:52 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 1, 2018, 1:52 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> addressed comments
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/8/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/
-----------------------------------------------------------

(Updated Jan. 1, 2018, 1:52 a.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
-------

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


addressed comments


Diffs (updated)
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/8/

Changes: https://reviews.apache.org/r/61172/diff/7-8/


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



FAIL: Some Mesos tests failed.

Reviews applied: `['61172']`

Failed command: `D:\DCOS\mesos\src\mesos-tests.exe --verbose`

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

Relevant logs:

- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61172/logs/mesos-tests-stdout.log):

```

[----------] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN      ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[       OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2300 ms)
[----------] 1 test from IsolationFlag/CpuIsolatorTest (2321 ms total)

[----------] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN      ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[       OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2360 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (2381 ms total)

[----------] Global test environment tear-down
[==========] 835 tests from 85 test cases ran. (299416 ms total)
[  PASSED  ] 825 tests.
[  FAILED  ] 10 tests, listed below:
[  FAILED  ] OfferOperationStatusUpdateManagerTest.UpdateAndAckNonTerminalUpdate
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RecoverCheckpointedStream
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RecoverEmptyFile
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RecoverTerminatedStream
[  FAILED  ] OfferOperationStatusUpdateManagerTest.IgnoreDuplicateUpdate
[  FAILED  ] OfferOperationStatusUpdateManagerTest.IgnoreDuplicateUpdateAfterRecover
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RejectDuplicateAck
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RejectDuplicateAckAfterRecover
[  FAILED  ] OfferOperationStatusUpdateManagerTest.NonStrictRecoveryCorruptedFile
[  FAILED  ] OfferOperationStatusUpdateManagerTest.UpdateLatestWhenResending

10 FAILED TESTS
  YOU HAVE 205 DISABLED TESTS

```

- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61172/logs/mesos-tests-stderr.log):

```
I1215 00:54:40.913776  4288 executor.cpp:171] Received SUBSCRIBED event
I1215 00:54:40.917809  4288 executor.cpp:175] Subscribed executor on build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I1215 00:54:40.917809  4288 executor.cpp:171] Received LAUNCH event
I1215 00:54:40.921775  4288 executor.cpp:638] Starting task 0662bbfd-d2a8-4c5d-a66c-f03db05a338c
I1215 00:54:41.002766  4288 executor.cpp:478] Running 'D:\DCOS\mesos\src\mesos-containerizer.exe launch <POSSIBLY-SENSITIVE-DATA>'
I1215 00:54:41.522768  4288 executor.cpp:651] Forked command at 7960
I1215 00:54:41.549741  3068 exec.cpp:435] Executor asked to shutdown
I1215 00:54:41.550741  6000 executor.cpp:171] Received SHUTDOWN event
I1215 00:54:41.550741  6000 executor.cpp:748] Shutting down
I1215 00:54:41.550741  6000 executor.cpp:855] Sending SIGTERM to process tree at pid 7f1d-620f715674fc@10.3.1.5:55494
I1215 00:54:41.548761  3324 hierarchical.cpp:405] Deactivated framework 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-0000
I1215 00:54:41.548761  6532 master.cpp:10160] Updating the state of task 0662bbfd-d2a8-4c5d-a66c-f03db05a338c of framework 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I1215 00:54:41.548761  4976 slave.cpp:3401] Shutting down framework 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-0000
I1215 00:54:41.548761  4976 slave.cpp:6109] Shutting down executor '0662bbfd-d2a8-4c5d-a66c-f03db05a338c' of framework 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-0000 at executor(1)@10.3.1.5:55515
I1215 00:54:41.549741  4976 slave.cpp:909] Agent terminating
W1215 00:54:41.549741  4976 slave.cpp:3397] Ignoring shutdown framework 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-0000 because it is terminating
I1215 00:54:41.550741  6532 master.cpp:10266] Removing task 0662bbfd-d2a8-4c5d-a66c-f03db05a338c with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-0000 on agent 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-S0 at slave(327)@10.3.1.5:55494 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I1215 00:54:41.552742  3324 containerizer.cpp:2337] Destroying container 265f8346-5ca3-4a3e-a170-6fc4179b8f8c in RUNNING state
I1215 00:54:41.552742  3324 containerizer.cpp:2939] Transitioning the state of container 265f8346-5ca3-4a3e-a170-6fc4179b8f8c from RUNNING to DESTROYING
I1215 00:54:41.553786  3324 launcher.cpp:156] Asked to destroy container 265f8346-5ca3-4a3e-a170-6fc4179b8f8c
I1215 00:54:41.554741  6532 master.cpp:1305] Agent 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-S0 at slave(327)@10.3.1.5:55494 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I1215 00:54:41.554741  6532 master.cpp:3364] Disconnecting agent 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-S0 at slave(327)@10.3.1.5:55494 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I1215 00:54:41.554741  5304 hierarchical.cpp:344] Removed framework 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-0000
I1215 00:54:41.555742  6532 master.cpp:3383] Deactivating agent 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-S0 at slave(327)@10.3.1.5:55494 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I1215 00:54:41.555742  2020 hierarchical.cpp:766] Agent 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-S0 deactivated
I1215 00:54:41.655813  5304 containerizer.cpp:2788] Container 265f8346-5ca3-4a3e-a170-6fc4179b8f8c has exited
I1215 00:54:41.683852  6620 master.cpp:1147] Master terminating
I1215 00:54:41.685817  5304 hierarchical.cpp:609] Removed agent 2bd92c1d-99e0-4d10-bd5b-6f3e1ed566ec-S0
I1215 00:54:42.003293  4012 process.cpp:887] Failed to accept socket: future discarded
```

- Mesos Reviewbot Windows


On Nov. 30, 2017, 10:16 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2017, 10:16 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/7/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/
-----------------------------------------------------------

(Updated Nov. 30, 2017, 10:16 a.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
-------

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


Diffs (updated)
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/7/

Changes: https://reviews.apache.org/r/61172/diff/6-7/


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



Patch looks great!

Reviews applied: [61172]

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 Sept. 29, 2017, 9:25 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2017, 9:25 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   src/python/pylint.config 9f0361d160fa82038ccefcbc146e52f0af8f99e8 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/6/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/
-----------------------------------------------------------

(Updated Sept. 29, 2017, 9:25 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Changes
-------

rebase


Repository: mesos


Description
-------

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.


Diffs (updated)
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  src/python/pylint.config 9f0361d160fa82038ccefcbc146e52f0af8f99e8 


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

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


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



Patch looks great!

Reviews applied: [61172]

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 Sept. 21, 2017, 4:27 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 4:27 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/5/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review186685
-----------------------------------------------------------




src/python/lib/mesos/exceptions.py
Lines 21 (patched)
<https://reviews.apache.org/r/61172/#comment263477>

    We don't need this because we don't import anything.



src/python/lib/mesos/exceptions.py
Lines 50 (patched)
<https://reviews.apache.org/r/61172/#comment263479>

    Please reword as "Could not fetch..." All exceptions will be wrapped with an "Error" when caught at a higher level.



src/python/lib/mesos/exceptions.py
Lines 69 (patched)
<https://reviews.apache.org/r/61172/#comment263481>

    Ditto



src/python/lib/mesos/http.py
Lines 33 (patched)
<https://reviews.apache.org/r/61172/#comment263484>

    Typically we separate these out one per line for consistency



src/python/lib/mesos/http.py
Lines 249 (patched)
<https://reviews.apache.org/r/61172/#comment263491>

    What is mesos specific about this class to justify calling it `MesosResource`?



src/python/lib/tests/conftest.py
Lines 27 (patched)
<https://reviews.apache.org/r/61172/#comment263485>

    Until now we haven't really been using python decorators anywhere. It seems weird to introduce them here without having them anywhere else. Independently, why not run this test using the other test infrastructure we have for actually spinning up a master / agent?



src/python/lib/tests/test_exceptions.py
Lines 27 (patched)
<https://reviews.apache.org/r/61172/#comment263487>

    Again, it seems weird to introduce this without introducing it everywhere.



src/python/lib/tests/test_http.py
Lines 41 (patched)
<https://reviews.apache.org/r/61172/#comment263488>

    Ditto



src/python/lib/tests/test_http.py
Lines 124 (patched)
<https://reviews.apache.org/r/61172/#comment263490>

    In general, all of these tests should have the same look and feel as the existing tests. 
    
    If we think the way you have organized these is the right way to go, we can change the exisiting ones to follow this pattern, but we should strive for consistency in whatever we finally commit back.
    
    As of now, it's hard for me to glance through these patches and knwo whether everything is correct or not because I am not familiar with the style you are going for - it's different than the style used throughout the rest of the code base.


- Kevin Klues


On Sept. 21, 2017, 4:27 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 4:27 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/5/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review186680
-----------------------------------------------------------



Looks like this needs a rebase. I can't get it to apply cleanly.

- Kevin Klues


On Sept. 21, 2017, 4:27 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 4:27 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/5/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



PASS: Mesos patch 61172 was successfully built and tested.

Reviews applied: `['61172']`

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

- Mesos Reviewbot Windows


On Sept. 21, 2017, 4:27 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 4:27 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/5/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

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



FAIL: Some Mesos tests failed.

Reviews applied: `['61172']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose --gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`

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

Relevant logs:

- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61172/logs/mesos-tests-stdout.log):

```
[ RUN      ] MasterMaintenanceTest.InverseOffers
[       OK ] MasterMaintenanceTest.InverseOffers (1203 ms)
[ RUN      ] MasterMaintenanceTest.InverseOffersFilters
[       OK ] MasterMaintenanceTest.InverseOffersFilters (1698 ms)
[ RUN      ] MasterMaintenanceTest.EndpointsBadAuthentication
[       OK ] MasterMaintenanceTest.EndpointsBadAuthentication (420 ms)
[ RUN      ] MasterMaintenanceTest.AcceptInvalidInverseOffer
[       OK ] MasterMaintenanceTest.AcceptInvalidInverseOffer (271 ms)
[----------] 12 tests from MasterMaintenanceTest (8944 ms total)

[----------] 7 tests from MasterSlaveReconciliationTest
[ RUN      ] MasterSlaveReconciliationTest.SlaveReregisterTerminatedExecutor
[       OK ] MasterSlaveReconciliationTest.SlaveReregisterTerminatedExecutor (871 ms)
[ RUN      ] MasterSlaveReconciliationTest.ReconcileLostTask
[       OK ] MasterSlaveReconciliationTest.ReconcileLostTask (1089 ms)
[ RUN      ] MasterSlaveReconciliationTest.ReconcileDroppedTask
[       OK ] MasterSlaveReconciliationTest.ReconcileDroppedTask (1081 ms)
[ RUN      ] MasterSlaveReconciliationTest.ReconcileRace
[       OK ] MasterSlaveReconciliationTest.ReconcileRace (900 ms)
[ RUN      ] MasterSlaveReconciliationTest.SlaveReregisterPendingTask
[       OK ] MasterSlaveReconciliationTest.SlaveReregisterPendingTask (676 ms)
[ RUN      ] MasterSlaveReconciliationTest.SlaveReregisterTerminalTask
[       OK ] MasterSlaveReconciliationTest.SlaveReregisterTerminalTask (901 ms)
[ RUN      ] MasterSlaveReconciliationTest.SlaveReregisterFrameworks
[       OK ] MasterSlaveReconciliationTest.SlaveReregisterFrameworks (794 ms)
[----------] 7 tests from MasterSlaveReconciliationTest (6634 ms total)

[----------] 5 tests from PartitionTest
[ RUN      ] PartitionTest.PartitionedSlave
```

- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61172/logs/mesos-tests-stderr.log):

```
I0921 06:13:49.891486 21388 slave.cpp:1000] New master detected at master@10.3.1.7:61110
I0921 06:13:49.898681 21388 slave.cpp:1024] No credentials provided. Attempting to register without authentication
I0921 06:13:49.898681 21388 slave.cpp:1035] Detecting new master
I0921 06:13:49.898681 20056 master.cpp:5534] Processing ACKNOWLEDGE call bc19cb56-6984-470a-b549-e0c8ed27ac21 for task 1 of framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 (default) at scheduler-445549ea-c875-4c1a-8e3f-4137c1ce13ab@10.3.1.7:61110 on agent 24774f80-5e0f-406c-a2f6-0e69040f4f40-S0
W0921 06:13:49.900687 18344 slave.cpp:3649] Dropping status update acknowledgement message for 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 because the agent is in DISCONNECTED state
I0921 06:13:49.912688 17940 sched.cpp:2021] Asked to stop the driver
I0921 06:13:49.912688 20056 sched.cpp:1203] Stopping framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000
I0921 06:13:49.912688 15820 master.cpp:6069] Received re-register agent message from agent 24774f80-5e0f-406c-a2f6-0e69040f4f40-S0 at slave(64)@10.3.1.7:61110 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0921 06:13:49.923689 15820 master.cpp:3806] Authorizing agent without a principal
I0921 06:13:49.925688 15820 master.cpp:8406] Processing TEARDOWN call for framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 (default) at scheduler-445549ea-c875-4c1a-8e3f-4137c1ce13ab@10.3.1.7:61110
I0921 06:13:49.925688 15820 master.cpp:8418] Removing framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 (default) at scheduler-445549ea-c875-4c1a-8e3f-4137c1ce13ab@10.3.1.7:61110
I0921 06:13:49.925688 15820 master.cpp:3267] Deactivating framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 (default) at scheduler-445549ea-c875-4c1a-8e3f-4137c1ce13ab@10.3.1.7:61110
I0921 06:13:49.926688 20056 hierarchical.cpp:412] Deactivated framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000
I0921 06:13:49.926688 15820 master.cpp:8993] Updating the state of task 1 of framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
W0921 06:13:49.927687 18344 slave.cpp:3229] Ignoring shutdown framework message for 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 because the agent has not yet registered with the master
I0921 06:13:49.937690 18344 slave.cpp:867] Agent terminating
W0921 06:13:49.938688 18344 slave.cpp:3229] Ignoring shutdown framework message for 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 because the agent has not yet registered with the master
I0921 06:13:49.941689 15820 master.cpp:9087] Removing task 1 with resources [{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}] of framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 on agent 24774f80-5e0f-406c-a2f6-0e69040f4f40-S0 at slave(64)@10.3.1.7:61110 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0921 06:13:49.965688 15820 master.cpp:9116] Removing executor 'default' with resources [] of framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000 on agent 24774f80-5e0f-406c-a2f6-0e69040f4f40-S0 at slave(64)@10.3.1.7:61110 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0921 06:13:49.967690 15820 master.cpp:1163] Master terminating
I0921 06:13:49.968688 19264 hierarchical.cpp:355] Removed framework 24774f80-5e0f-406c-a2f6-0e69040f4f40-0000
I0921 06:13:49.972688 19264 hierarchical.cpp:626] Removed agent 24774f80-5e0f-406c-a2f6-0e69040f4f40-S0
I0921 06:13:50.106498 17940 cluster.cpp:162] Creating default 'local' authorizer
I0921 06:13:50.130498 20056 master.cpp:445] Master 9a79ab85-f9ae-4532-a6de-f33a3f860a94 (mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) started on 10.3.1.7:61110
I0921 06:13:50.151499 20056 master.cpp:447] Flags at startup: --acls="" --agent_ping_timeout="15secs" --agent_reregister_timeout="10mins" --allocation_interval="1secs" --allocator="HierarchicalDRF" --authenticate_agents="false" --authenticate_frameworks="false" --authenticate_http_frameworks="true" --authenticate_http_readonly="true" --authenticate_http_readwrite="true" --authenticators="crammd5" --authorizers="local" --credentials="C:\Users\mesos\AppData\Local\Temp\2\gis0bL\credentials" --filter_gpu_resources="true" --framework_sorter="drf" --help="false" --hostname_lookup="true" --http_authenticators="basic" --http_framework_authenticators="basic" --initialize_driver_logging="true" --log_auto_initialize="true" --logbufsecs="0" --logging_level="INFO" --max_agent_ping_timeouts="5" --max_completed_frameworks="50" --max_completed_tasks_per_framework="1000" --max_unreachable_tasks_per_framework="1000" --port="5050" --quiet="false" --recovery_agent_removal_limit="100%" --registry="in_mem
 ory" --registry_fetch_timeout="1mins" --registry_gc_interval="15mins" --registry_max_agent_age="2weeks" --registry_max_agent_count="102400" --registry_store_timeout="100secs" --registry_strict="false" --root_submissions="true" --user_sorter="drf" --version="false" --webui_dir="/webui" --work_dir="C:\Users\mesos\AppData\Local\Temp\2\gis0bL\master" --zk_session_timeout="10secs"
I0921 06:13:50.169566 20056 master.cpp:524] Master only allowing authenticated HTTP frameworks to register
I0921 06:13:50.169566 20056 credentials.hpp:37] Loading credentials for authentication from 'C:\Users\mesos\AppData\Local\Temp\2\gis0bL\credentials'
I0921 06:13:50.172499 20056 http.cpp:1045] Creating default 'basic' HTTP authenticator for realm 'mesos-master-readonly'
I0921 06:13:50.172499 20056 http.cpp:1045] Creating default 'basic' HTTP authenticator for realm 'mesos-master-readwrite'
I0921 06:13:50.173499 20056 http.cpp:1045] Creating default 'basic' HTTP authenticator for rea```

- Mesos Reviewbot Windows


On Sept. 20, 2017, 9:27 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2017, 9:27 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/5/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

Posted by Eric Chung <ci...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/
-----------------------------------------------------------

(Updated Sept. 21, 2017, 4:27 a.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description
-------

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.


Diffs (updated)
-----

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 


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

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


Testing
-------

install tox
cd src/python/lib
tox


Thanks,

Eric Chung