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/07/07 01:28:15 UTC

Review Request 60697: Added mesos.http module for abstracting out http requests in the new cli.

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

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


Repository: mesos


Description
-------

Part of MESOS-7310, this patch adds the mesos.http module to the src/python/lib/mesos
package. This module will encapsulate the details in making an http request, such as
auth scheme, default headers, timeouts, and so on. Retries will be added in subsequent diffs.


Diffs
-----

  src/python/lib/mesos/http.py PRE-CREATION 


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


Testing
-------


Thanks,

Eric Chung


Re: Review Request 60697: Added mesos.http module for abstracting out http requests in the new cli.

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




src/python/lib/mesos/http.py
Lines 35 (patched)
<https://reviews.apache.org/r/60697/#comment254762>

    According to `pep257`, the first line should be in imperative mood and end with a period. There should be one blank line between this summary line and the rest of the description.



src/python/lib/mesos/http.py
Lines 48 (patched)
<https://reviews.apache.org/r/60697/#comment254763>

    Same comment as the one before.



src/python/lib/mesos/http.py
Lines 55 (patched)
<https://reviews.apache.org/r/60697/#comment254764>

    The first line of the docstring should describe anything that you cannot tell from the method's signature. E.g. the choice of using a frozenset for the default headers.



src/python/lib/mesos/http.py
Lines 84 (patched)
<https://reviews.apache.org/r/60697/#comment254765>

    You can drop `current` and then have a summary line.



src/python/lib/mesos/http.py
Lines 102 (patched)
<https://reviews.apache.org/r/60697/#comment254766>

    A period is missing. The method is described as a token and not as a verb in https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html.


- Armand Grillet


On July 7, 2017, 1:28 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60697/
> -----------------------------------------------------------
> 
> (Updated July 7, 2017, 1:28 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 module to the src/python/lib/mesos
> package. This module will encapsulate the details in making an http request, such as
> auth scheme, default headers, timeouts, and so on. Retries will be added in subsequent diffs.
> 
> 
> Diffs
> -----
> 
>   src/python/lib/mesos/http.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60697/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Re: Review Request 60697: Added mesos.http module for abstracting out http requests in the new cli.

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



Patch looks great!

Reviews applied: [60697]

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 July 7, 2017, 1:28 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60697/
> -----------------------------------------------------------
> 
> (Updated July 7, 2017, 1:28 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 module to the src/python/lib/mesos
> package. This module will encapsulate the details in making an http request, such as
> auth scheme, default headers, timeouts, and so on. Retries will be added in subsequent diffs.
> 
> 
> Diffs
> -----
> 
>   src/python/lib/mesos/http.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60697/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eric Chung
> 
>