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/12/14 22:43:14 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/#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
> 
>