You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2015/03/26 17:32:59 UTC

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

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



3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/30032/#comment126242>

    One thing captures my attention is how we include C headers. AFAIK, the standard requires to include them like
    ```
    #include <ctime>
    #include <climits>
    #include <cstdio>
    ```
    and so on.
    
    Could you please create a cleanup newbie JIRA for this?


- Alexander Rukletsov


On March 26, 2015, 9:53 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30032/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 9:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache.
> When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide.
> Unit tests added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 9cf05ac 
>   3rdparty/libprocess/src/process.cpp 67b6b3b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a 
> 
> Diff: https://reviews.apache.org/r/30032/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

Posted by Alexander Rojas <al...@mesosphere.io>.

> On March 26, 2015, 5:32 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, line 6
> > <https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line6>
> >
> >     One thing captures my attention is how we include C headers. AFAIK, the standard requires to include them like
> >     ```
> >     #include <ctime>
> >     #include <climits>
> >     #include <cstdio>
> >     ```
> >     and so on.
> >     
> >     Could you please create a cleanup newbie JIRA for this?
> 
> Alexander Rojas wrote:
>     Not very sure about that, what it does by using the c… versions of the headers is to put its functions in the std namespace. Check the C++11 Standard annex *D.6 C standard library headers*:
>     
>     > 2 Every C header, each of which has a name of the form name.h, behaves as if each name placed in the standard library namespace by the corresponding cname header is placed within the global namespace scope. It is unspecified whether these names are first declared or defined within namespace scope (3.3.6) of the namespace std and are then injected into the global namespace scope by explicit using-declarations (7.3.3).
>     >
>     > 3 Example: The header <cstdlib> assuredly provides its declarations and definitions within the namespace std. It may also provide these names within the global namespace. The header <stdlib.h> assuredly provides the same declarations and definitions within the global namespace, much as in the C Standard. It may also provide these names within the namespace std. —end example.
>     
>     
>     As it reads, using functions from the `cname` versions of the `name.h` headers, may or may not (stdlib implementation specific) require to add the namespace std too all functions and structs provided by the header.
> 
> Till Toenshoff wrote:
>     Seems we got three options here;
>     - start using the C++ wrappers and allowed for a smooth transition (file by file as needed/touched)
>     - - not sure what the implications here are from an optimizing (linker) point of view
>     - global cleanup and `std` namespace added everywhere
>     - stick with the C variants
>     
>     May be worth a dev-list discussion?
> 
> Alexander Rukletsov wrote:
>     C++11 Standard 17.6.1.2 p4, p8, and reference 177 hint at a preferred form for C++ programs.

I think the best would be to add a discussion on the dev list, since changing to the `cname` version would require to prefix every call with the prefix `std::`


- Alexander


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


On March 26, 2015, 10:53 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30032/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 10:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache.
> When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide.
> Unit tests added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 9cf05ac 
>   3rdparty/libprocess/src/process.cpp 67b6b3b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a 
> 
> Diff: https://reviews.apache.org/r/30032/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

Posted by Till Toenshoff <to...@me.com>.

> On March 26, 2015, 4:32 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, line 6
> > <https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line6>
> >
> >     One thing captures my attention is how we include C headers. AFAIK, the standard requires to include them like
> >     ```
> >     #include <ctime>
> >     #include <climits>
> >     #include <cstdio>
> >     ```
> >     and so on.
> >     
> >     Could you please create a cleanup newbie JIRA for this?
> 
> Alexander Rojas wrote:
>     Not very sure about that, what it does by using the c… versions of the headers is to put its functions in the std namespace. Check the C++11 Standard annex *D.6 C standard library headers*:
>     
>     > 2 Every C header, each of which has a name of the form name.h, behaves as if each name placed in the standard library namespace by the corresponding cname header is placed within the global namespace scope. It is unspecified whether these names are first declared or defined within namespace scope (3.3.6) of the namespace std and are then injected into the global namespace scope by explicit using-declarations (7.3.3).
>     >
>     > 3 Example: The header <cstdlib> assuredly provides its declarations and definitions within the namespace std. It may also provide these names within the global namespace. The header <stdlib.h> assuredly provides the same declarations and definitions within the global namespace, much as in the C Standard. It may also provide these names within the namespace std. —end example.
>     
>     
>     As it reads, using functions from the `cname` versions of the `name.h` headers, may or may not (stdlib implementation specific) require to add the namespace std too all functions and structs provided by the header.

Seems we got three options here;
- start using the C++ wrappers and allowed for a smooth transition (file by file as needed/touched)
- - not sure what the implications here are from an optimizing (linker) point of view
- global cleanup and `std` namespace added everywhere
- stick with the C variants

May be worth a dev-list discussion?


- Till


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


On March 26, 2015, 9:53 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30032/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 9:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache.
> When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide.
> Unit tests added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 9cf05ac 
>   3rdparty/libprocess/src/process.cpp 67b6b3b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a 
> 
> Diff: https://reviews.apache.org/r/30032/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

Posted by Alexander Rojas <al...@mesosphere.io>.

> On March 26, 2015, 5:32 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, line 6
> > <https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line6>
> >
> >     One thing captures my attention is how we include C headers. AFAIK, the standard requires to include them like
> >     ```
> >     #include <ctime>
> >     #include <climits>
> >     #include <cstdio>
> >     ```
> >     and so on.
> >     
> >     Could you please create a cleanup newbie JIRA for this?

Not very sure about that, what it does by using the c… versions of the headers is to put its functions in the std namespace. Check the C++11 Standard annex *D.6 C standard library headers*:

> 2 Every C header, each of which has a name of the form name.h, behaves as if each name placed in the standard library namespace by the corresponding cname header is placed within the global namespace scope. It is unspecified whether these names are first declared or defined within namespace scope (3.3.6) of the namespace std and are then injected into the global namespace scope by explicit using-declarations (7.3.3).
>
> 3 Example: The header <cstdlib> assuredly provides its declarations and definitions within the namespace std. It may also provide these names within the global namespace. The header <stdlib.h> assuredly provides the same declarations and definitions within the global namespace, much as in the C Standard. It may also provide these names within the namespace std. —end example.


As it reads, using functions from the `cname` versions of the `name.h` headers, may or may not (stdlib implementation specific) require to add the namespace std too all functions and structs provided by the header.


- Alexander


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


On March 26, 2015, 10:53 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30032/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 10:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache.
> When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide.
> Unit tests added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 9cf05ac 
>   3rdparty/libprocess/src/process.cpp 67b6b3b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a 
> 
> Diff: https://reviews.apache.org/r/30032/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

Posted by Alexander Rojas <al...@mesosphere.io>.

> On March 26, 2015, 5:32 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, line 6
> > <https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line6>
> >
> >     One thing captures my attention is how we include C headers. AFAIK, the standard requires to include them like
> >     ```
> >     #include <ctime>
> >     #include <climits>
> >     #include <cstdio>
> >     ```
> >     and so on.
> >     
> >     Could you please create a cleanup newbie JIRA for this?
> 
> Alexander Rojas wrote:
>     Not very sure about that, what it does by using the c… versions of the headers is to put its functions in the std namespace. Check the C++11 Standard annex *D.6 C standard library headers*:
>     
>     > 2 Every C header, each of which has a name of the form name.h, behaves as if each name placed in the standard library namespace by the corresponding cname header is placed within the global namespace scope. It is unspecified whether these names are first declared or defined within namespace scope (3.3.6) of the namespace std and are then injected into the global namespace scope by explicit using-declarations (7.3.3).
>     >
>     > 3 Example: The header <cstdlib> assuredly provides its declarations and definitions within the namespace std. It may also provide these names within the global namespace. The header <stdlib.h> assuredly provides the same declarations and definitions within the global namespace, much as in the C Standard. It may also provide these names within the namespace std. —end example.
>     
>     
>     As it reads, using functions from the `cname` versions of the `name.h` headers, may or may not (stdlib implementation specific) require to add the namespace std too all functions and structs provided by the header.
> 
> Till Toenshoff wrote:
>     Seems we got three options here;
>     - start using the C++ wrappers and allowed for a smooth transition (file by file as needed/touched)
>     - - not sure what the implications here are from an optimizing (linker) point of view
>     - global cleanup and `std` namespace added everywhere
>     - stick with the C variants
>     
>     May be worth a dev-list discussion?
> 
> Alexander Rukletsov wrote:
>     C++11 Standard 17.6.1.2 p4, p8, and reference 177 hint at a preferred form for C++ programs.
> 
> Alexander Rojas wrote:
>     I think the best would be to add a discussion on the dev list, since changing to the `cname` version would require to prefix every call with the prefix `std::`
> 
> Alexander Rojas wrote:
>     Preferred is a very different thing from required, as mention originally. And I do agree that I prefer in my private projects to use the `cname` versions. However, I wouldn't do that since it has the capacity of becoming red herring.

Droping because the discusion when nowhere, so we keep the status quo.


- Alexander


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


On March 26, 2015, 10:53 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30032/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 10:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache.
> When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide.
> Unit tests added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 9cf05ac 
>   3rdparty/libprocess/src/process.cpp 67b6b3b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a 
> 
> Diff: https://reviews.apache.org/r/30032/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On March 26, 2015, 4:32 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, line 6
> > <https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line6>
> >
> >     One thing captures my attention is how we include C headers. AFAIK, the standard requires to include them like
> >     ```
> >     #include <ctime>
> >     #include <climits>
> >     #include <cstdio>
> >     ```
> >     and so on.
> >     
> >     Could you please create a cleanup newbie JIRA for this?
> 
> Alexander Rojas wrote:
>     Not very sure about that, what it does by using the c… versions of the headers is to put its functions in the std namespace. Check the C++11 Standard annex *D.6 C standard library headers*:
>     
>     > 2 Every C header, each of which has a name of the form name.h, behaves as if each name placed in the standard library namespace by the corresponding cname header is placed within the global namespace scope. It is unspecified whether these names are first declared or defined within namespace scope (3.3.6) of the namespace std and are then injected into the global namespace scope by explicit using-declarations (7.3.3).
>     >
>     > 3 Example: The header <cstdlib> assuredly provides its declarations and definitions within the namespace std. It may also provide these names within the global namespace. The header <stdlib.h> assuredly provides the same declarations and definitions within the global namespace, much as in the C Standard. It may also provide these names within the namespace std. —end example.
>     
>     
>     As it reads, using functions from the `cname` versions of the `name.h` headers, may or may not (stdlib implementation specific) require to add the namespace std too all functions and structs provided by the header.
> 
> Till Toenshoff wrote:
>     Seems we got three options here;
>     - start using the C++ wrappers and allowed for a smooth transition (file by file as needed/touched)
>     - - not sure what the implications here are from an optimizing (linker) point of view
>     - global cleanup and `std` namespace added everywhere
>     - stick with the C variants
>     
>     May be worth a dev-list discussion?

C++11 Standard 17.6.1.2 p4, p8, and reference 177 hint at a preferred form for C++ programs.


- Alexander


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


On March 26, 2015, 9:53 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30032/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 9:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache.
> When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide.
> Unit tests added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 9cf05ac 
>   3rdparty/libprocess/src/process.cpp 67b6b3b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a 
> 
> Diff: https://reviews.apache.org/r/30032/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

Posted by Alexander Rojas <al...@mesosphere.io>.

> On March 26, 2015, 5:32 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, line 6
> > <https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line6>
> >
> >     One thing captures my attention is how we include C headers. AFAIK, the standard requires to include them like
> >     ```
> >     #include <ctime>
> >     #include <climits>
> >     #include <cstdio>
> >     ```
> >     and so on.
> >     
> >     Could you please create a cleanup newbie JIRA for this?
> 
> Alexander Rojas wrote:
>     Not very sure about that, what it does by using the c… versions of the headers is to put its functions in the std namespace. Check the C++11 Standard annex *D.6 C standard library headers*:
>     
>     > 2 Every C header, each of which has a name of the form name.h, behaves as if each name placed in the standard library namespace by the corresponding cname header is placed within the global namespace scope. It is unspecified whether these names are first declared or defined within namespace scope (3.3.6) of the namespace std and are then injected into the global namespace scope by explicit using-declarations (7.3.3).
>     >
>     > 3 Example: The header <cstdlib> assuredly provides its declarations and definitions within the namespace std. It may also provide these names within the global namespace. The header <stdlib.h> assuredly provides the same declarations and definitions within the global namespace, much as in the C Standard. It may also provide these names within the namespace std. —end example.
>     
>     
>     As it reads, using functions from the `cname` versions of the `name.h` headers, may or may not (stdlib implementation specific) require to add the namespace std too all functions and structs provided by the header.
> 
> Till Toenshoff wrote:
>     Seems we got three options here;
>     - start using the C++ wrappers and allowed for a smooth transition (file by file as needed/touched)
>     - - not sure what the implications here are from an optimizing (linker) point of view
>     - global cleanup and `std` namespace added everywhere
>     - stick with the C variants
>     
>     May be worth a dev-list discussion?
> 
> Alexander Rukletsov wrote:
>     C++11 Standard 17.6.1.2 p4, p8, and reference 177 hint at a preferred form for C++ programs.
> 
> Alexander Rojas wrote:
>     I think the best would be to add a discussion on the dev list, since changing to the `cname` version would require to prefix every call with the prefix `std::`

Preferred is a very different thing from required, as mention originally. And I do agree that I prefer in my private projects to use the `cname` versions. However, I wouldn't do that since it has the capacity of becoming red herring.


- Alexander


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


On March 26, 2015, 10:53 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30032/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 10:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache.
> When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide.
> Unit tests added.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 9cf05ac 
>   3rdparty/libprocess/src/process.cpp 67b6b3b 
>   3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a 
> 
> Diff: https://reviews.apache.org/r/30032/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>