You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2015/09/22 08:18:36 UTC
Review Request 38596: Updated http::URL to be a URI-reference.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38596/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
Repository: mesos
Description
-------
This updates URL to be a URI-reference (see [RFC 3986 4.1](https://tools.ietf.org/html/rfc3986#section-4.1)), similarly to how other http libraries store a URL in a Request object (see the go libraries for [Request](http://golang.org/pkg/net/http/#Request) and [URL](http://golang.org/pkg/net/url/#URL) as an example).
The motivation for this change is to enable cleanup of the http utilities in libprocess. By storing a full URL inside the Request, we can obtain the address from the request itself, rather than having to pass the address alongside.
Diffs
-----
3rdparty/libprocess/include/process/http.hpp fbd6cf7967173495875a8ea90ed28ade88b982a2
3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4
Diff: https://reviews.apache.org/r/38596/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 38596: Updated http::URL to be a URI-reference.
Posted by Ben Mahler <be...@gmail.com>.
> On Sept. 23, 2015, 8:50 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/http.cpp, line 879
> > <https://reviews.apache.org/r/38596/diff/1/?file=1079691#file1079691line879>
> >
> > can url.port be None() in this method? looks like request() returns an error if that's the case. should it be a CHECK instead?
It cannot for now, but since None() may mean "use the default port for the scheme" and since we have the address passed in containing the port, it seemed appropriate to just handle it as being the default port for the scheme.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38596/#review100280
-----------------------------------------------------------
On Sept. 22, 2015, 6:18 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38596/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2015, 6:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This updates URL to be a URI-reference (see [RFC 3986 4.1](https://tools.ietf.org/html/rfc3986#section-4.1)), similarly to how other http libraries store a URL in a Request object (see the go libraries for [Request](http://golang.org/pkg/net/http/#Request) and [URL](http://golang.org/pkg/net/url/#URL) as an example).
>
> The motivation for this change is to enable cleanup of the http utilities in libprocess. By storing a full URL inside the Request, we can obtain the address from the request itself, rather than having to pass the address alongside.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp fbd6cf7967173495875a8ea90ed28ade88b982a2
> 3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4
>
> Diff: https://reviews.apache.org/r/38596/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 38596: Updated http::URL to be a URI-reference.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38596/#review100280
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/src/http.cpp (line 876)
<https://reviews.apache.org/r/38596/#comment157431>
can url.port be None() in this method? looks like request() returns an error if that's the case. should it be a CHECK instead?
- Vinod Kone
On Sept. 22, 2015, 6:18 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38596/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2015, 6:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This updates URL to be a URI-reference (see [RFC 3986 4.1](https://tools.ietf.org/html/rfc3986#section-4.1)), similarly to how other http libraries store a URL in a Request object (see the go libraries for [Request](http://golang.org/pkg/net/http/#Request) and [URL](http://golang.org/pkg/net/url/#URL) as an example).
>
> The motivation for this change is to enable cleanup of the http utilities in libprocess. By storing a full URL inside the Request, we can obtain the address from the request itself, rather than having to pass the address alongside.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp fbd6cf7967173495875a8ea90ed28ade88b982a2
> 3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4
>
> Diff: https://reviews.apache.org/r/38596/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>