[kudu-CR] [webui] Allow custom response codes and headers

2017-10-05 Thread Will Berkeley (Code Review) 
Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which have fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Reviewed-on: http://gerrit.cloudera.org:8080/8141
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 269 insertions(+), 137 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, but someone else must approve
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 13
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-05 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 12: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 12
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 05 Oct 2017 21:58:33 +
Gerrit-HasComments: No

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-04 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 12: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 12
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 04 Oct 2017 17:38:49 +
Gerrit-HasComments: No

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-04 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#12).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which have fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 269 insertions(+), 137 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/12
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 12
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-04 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#11).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which have fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 268 insertions(+), 136 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/11
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 11
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-04 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8141/10/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/8141/10/src/kudu/server/webserver.cc@474
PS10, Line 474: // Omit the header if it's one the webserver handles above.
This comment is no longer quite right since we crash instead of omitting.


http://gerrit.cloudera.org:8080/#/c/8141/10/src/kudu/server/webserver.cc@476
PS10, Line 476: overriden
Nit: overridden



--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 10
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 04 Oct 2017 16:44:03 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-04 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#10).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which have fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 268 insertions(+), 136 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/10
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 10
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-04 Thread Will Berkeley (Code Review) 
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h
File src/kudu/server/webserver.h:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h@66
PS6, Line 66:   // Register a route 'path' to be rendered via template.
> What about 'alias'?
Done


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@452
PS6, Line 452: HttpResponseHeaders{}
> Done
Actually seems it doesn't work in general (though it did on macos) -- see 
TidyBot's comment on PS7


http://gerrit.cloudera.org:8080/#/c/8141/8/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/8141/8/src/kudu/server/webserver.cc@472
PS8, Line 472:   headers_stream << "\r\n";
> Can use unordered_set which is hash based and slightly faster on average.
Done


http://gerrit.cloudera.org:8080/#/c/8141/8/src/kudu/server/webserver.cc@475
PS8, Line 475:   // Make sure to use sq_write for printing the body; sq_printf 
truncates at 8KB.
> Hmm, this seems too quiet: if a callback tries to send an invalid header, i
Done



--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 04 Oct 2017 09:08:38 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-04 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#9).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which have fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 263 insertions(+), 134 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/9
-- 
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-03 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h
File src/kudu/server/webserver.h:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h@66
PS6, Line 66:   // Register a route 'path' to be rendered via template.
> Done
What about 'alias'?


http://gerrit.cloudera.org:8080/#/c/8141/8/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/8141/8/src/kudu/server/webserver.cc@472
PS8, Line 472:   std::set invalid_headers{"Content-Type", 
"Content-Length", "X-Frame-Options"};
Can use unordered_set which is hash based and slightly faster on average.


http://gerrit.cloudera.org:8080/#/c/8141/8/src/kudu/server/webserver.cc@475
PS8, Line 475: if (ContainsKey(invalid_headers, entry.first)) continue;
Hmm, this seems too quiet: if a callback tries to send an invalid header, it'll 
just be silently ignored. I think it'd be better to crash, or to go in the 
other direction and allow callbacks to override those headers.



--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 03 Oct 2017 23:50:11 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-03 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#8).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which have fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 260 insertions(+), 134 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/8
-- 
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-03 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#7).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which have fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 260 insertions(+), 134 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/7
-- 
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-03 Thread Will Berkeley (Code Review) 
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 6:

(9 comments)

I let Impala people know about this patch.

http://gerrit.cloudera.org:8080/#/c/8141/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8141/6//COMMIT_MSG@14
PS6, Line 14: has
> have ("WebResponse and PrerenderedWebResponse" are plural)
Done


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/master/master-path-handlers.cc
File src/kudu/master/master-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/master/master-path-handlers.cc@242
PS6, Line 242: response
> respond
Done


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h
File src/kudu/server/webserver.h:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h@66
PS6, Line 66:   // Register a route 'path' to be rendered via template.
> Since you're already updating these docs, could you explain what 'alias', '
Done


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@88
PS6, Line 88: switch (code) {
> Add a default case that crashes or something? Would be good to know if we'v
Done, but I added outside the switch so the compiler will generate warnings for 
unhandled enum class members.


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@452
PS6, Line 452: HttpResponseHeaders{}
> Just {} doesn't work?
Done


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@469
PS6, Line 469:   for (const auto& entry : resp.response_headers) {
> Should we enforce that response_headers doesn't include Content-Type, Conte
Yeah, I think we should check for it. A repeated header is only allowed if the 
header value is CSV type, and none of those are.


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@486
PS6, Line 486: HttpResponseHeaders{}
> Just {} doesn't work?
Done


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h
File src/kudu/util/web_callback_registry.h:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h@66
PS6, Line 66:   typedef std::map HttpResponseHeaders;
> Is it important that the headers be sorted by key? Or can this be an unorde
Done


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h@69
PS6, Line 69:   // - 'status_code' determines the status code of the HTTP 
response.
:   // - 'response_headers' are additional headers added to the 
HTTP response.
:   // - 'output' is a JSON object to be rendered in a mustache 
template.
> Nit: move each of these so that it's just above the declaration of its fiel
Done



--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 03 Oct 2017 22:38:23 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-02 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 6:

(9 comments)

I'm not an HTTP expert so it'd be good to find a reviewer who is (maybe Todd?).

I know Impala also uses Squeasel; perhaps they even use this Webserver code. 
Could you check in with an Impala developer and see whether their Webserver 
already does this, or whether they'd be interested in this functionality?

http://gerrit.cloudera.org:8080/#/c/8141/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8141/6//COMMIT_MSG@14
PS6, Line 14: has
have ("WebResponse and PrerenderedWebResponse" are plural)


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/master/master-path-handlers.cc
File src/kudu/master/master-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/master/master-path-handlers.cc@242
PS6, Line 242: response
respond


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h
File src/kudu/server/webserver.h:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h@66
PS6, Line 66:   // Register a route 'path' to be rendered via template.
Since you're already updating these docs, could you explain what 'alias', 
'is_styled' and 'is_on_nav_bar' do?


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@88
PS6, Line 88: switch (code) {
Add a default case that crashes or something? Would be good to know if we've 
forgotten to update this switch when adding a new code.


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@452
PS6, Line 452: HttpResponseHeaders{}
Just {} doesn't work?


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@469
PS6, Line 469:   for (const auto& entry : resp.response_headers) {
Should we enforce that response_headers doesn't include Content-Type, 
Content-Length, or X-Frame-Options? I understand it's sort of moot since no 
callback responds with headers right now, but what happens if a duplicate 
header is in the resposne?


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@486
PS6, Line 486: HttpResponseHeaders{}
Just {} doesn't work?


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h
File src/kudu/util/web_callback_registry.h:

http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h@66
PS6, Line 66:   typedef std::map HttpResponseHeaders;
Is it important that the headers be sorted by key? Or can this be an 
unordered_map instead?


http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h@69
PS6, Line 69:   // - 'status_code' determines the status code of the HTTP 
response.
:   // - 'response_headers' are additional headers added to the 
HTTP response.
:   // - 'output' is a JSON object to be rendered in a mustache 
template.
Nit: move each of these so that it's just above the declaration of its field.

Same for 'output' in PrerenderedWebResponse.



--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 02 Oct 2017 21:33:14 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Allow custom response codes and headers

2017-10-02 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#6).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which has fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 247 insertions(+), 132 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/6
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-29 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#5).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which has fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 246 insertions(+), 132 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/5
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-29 Thread Will Berkeley (Code Review) 
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 4:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8141/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8141/4//COMMIT_MSG@17
PS4, Line 17: The ability to add headers isn't currently used, but it's nice to 
have.
> Do you expect these to be used in the near future?
I don't have immediate plans for it, but it's easy to add now as part of the 
WebResponse. Originally I used it to add a Location header for an automatic 
redirect when returning 307, but, as I explained in a comment later, that 
seemed like it would be confusing.


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/master/master-path-handlers.cc
File src/kudu/master/master-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/master/master-path-handlers.cc@243
PS4, Line 243: will
> nit: s/will/would (to avoid confusion around what is actually implemented)
Done


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/master/master-path-handlers.cc@242
PS4, Line 242: We could respond with 307 Temporary Redirect and automatically 
redirect with
 : // a Location header, but this will redirect without warning 
and users are liable
 : // to becomes confused about which master's web ui they are 
looking at.
> Is this a TODO or just a comment? If it's just a comment, maybe append some
Done


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/server/pprof-path-handlers.cc
File src/kudu/server/pprof-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/server/pprof-path-handlers.cc@87
PS4, Line 87: static void PprofHeapHandler(const Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Spurious b/c of macros.


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/server/pprof-path-handlers.cc@90
PS4, Line 90: *resp->output
> In functions that refer to resp->output a few times, maybe pull out the ost
Done


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/server/pprof-path-handlers.cc@117
PS4, Line 117: static void PprofCpuProfileHandler(const Webserver::WebRequest& 
req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Spurious b/c of macros.


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:

http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/util/thread.cc@224
PS4, Line 224:   void ThreadPathHandler(const WebCallbackRegistry::WebRequest& 
req,
> warning: function 'kudu::ThreadMgr::ThreadPathHandler' has a definition wit
Done


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/util/web_callback_registry.h
File src/kudu/util/web_callback_registry.h:

http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/util/web_callback_registry.h@70
PS4, Line 70: headers
> nit: response_headers
Done



--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Sat, 30 Sep 2017 00:18:23 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-29 Thread Andrew Wong (Code Review) 
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8141/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8141/4//COMMIT_MSG@17
PS4, Line 17: The ability to add headers isn't currently used, but it's nice to 
have.
Do you expect these to be used in the near future?


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/master/master-path-handlers.cc
File src/kudu/master/master-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/master/master-path-handlers.cc@243
PS4, Line 243: will
nit: s/will/would (to avoid confusion around what is actually implemented)


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/master/master-path-handlers.cc@242
PS4, Line 242: We could respond with 307 Temporary Redirect and automatically 
redirect with
 : // a Location header, but this will redirect without warning 
and users are liable
 : // to becomes confused about which master's web ui they are 
looking at.
Is this a TODO or just a comment? If it's just a comment, maybe append 
something like "...Instead, we simply point users to the current leader."


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/server/pprof-path-handlers.cc
File src/kudu/server/pprof-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/server/pprof-path-handlers.cc@90
PS4, Line 90: *resp->output
In functions that refer to resp->output a few times, maybe pull out the 
ostringstream*?


http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/util/web_callback_registry.h
File src/kudu/util/web_callback_registry.h:

http://gerrit.cloudera.org:8080/#/c/8141/4/src/kudu/util/web_callback_registry.h@70
PS4, Line 70: headers
nit: response_headers



--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 29 Sep 2017 18:36:41 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-28 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#4).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which has fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 237 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/4
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-27 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#3).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which has fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 243 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/3
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-26 Thread Will Berkeley (Code Review) 
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8141 )

Change subject: [webui] Allow custom response codes and headers
..


Patch Set 1:

(20 comments)

http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/master/master-path-handlers.cc
File src/kudu/master/master-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/master/master-path-handlers.cc@85
PS1, Line 85: void MasterPathHandlers::HandleTabletServers(const 
Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/master/master-path-handlers.cc@386
PS1, Line 386: void MasterPathHandlers::HandleMasters(const 
Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/master/master-path-handlers.cc@528
PS1, Line 528: void MasterPathHandlers::HandleDumpEntities(const 
Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc
File src/kudu/server/pprof-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@76
PS1, Line 76: static void PprofCmdLineHandler(const Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@87
PS1, Line 87: static void PprofHeapHandler(const Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@88
PS1, Line 88:  Webserver::PrerenderedWebResponse* 
resp) {
> warning: parameter 'resp' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@90
PS1, Line 90:   (*output) << "Heap profiling is not available without 
tcmalloc.";
> error: use of undeclared identifier 'output' [clang-diagnostic-error]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@117
PS1, Line 117: static void PprofCpuProfileHandler(const Webserver::WebRequest& 
req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@118
PS1, Line 118:
Webserver::PrerenderedWebResponse* resp) {
> warning: parameter 'resp' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@120
PS1, Line 120:   (*output) << "CPU profiling is not available without 
tcmalloc.";
> error: use of undeclared identifier 'output' [clang-diagnostic-error]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@145
PS1, Line 145: static void PprofGrowthHandler(const Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@146
PS1, Line 146:
Webserver::PrerenderedWebResponse* resp) {
> warning: parameter 'resp' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/pprof-path-handlers.cc@148
PS1, Line 148:   (*output) << "Growth profiling is not available without 
tcmalloc.";
> error: use of undeclared identifier 'output' [clang-diagnostic-error]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/webserver.cc@383
PS1, Line 383:   } else {
> warning: do not use 'else' after 'return' [readability-else-after-return]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/server/webserver.cc@420
PS1, Line 420:   // TODO: for this and other HTTP requests, we should log 
the
> warning: missing username/bug in TODO [google-readability-todo]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/tserver/tserver-path-handlers.cc
File src/kudu/tserver/tserver-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/tserver/tserver-path-handlers.cc@216
PS1, Line 216: void TabletServerPathHandlers::HandleTabletsPage(const 
Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/tserver/tserver-path-handlers.cc@378
PS1, Line 378: bool GetTabletReplica(TabletServer* tserver, const 
Webserver::WebRequest& req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/8141/1/src/kudu/tserver/tserver-path-handlers.cc@509
PS1, Line 509: void

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-26 Thread Will Berkeley (Code Review) 
Hello Tidy Bot, Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8141

to look at the new patch set (#2).

Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which has fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 239 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/2
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [webui] Allow custom response codes and headers

2017-09-26 Thread Will Berkeley (Code Review) 
Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8141


Change subject: [webui] Allow custom response codes and headers
..

[webui] Allow custom response codes and headers

Previously, the path handlers used to implement pages in the web ui
could only return 200 OK and could not set any headers, as these two
aspects of the HTTP response were handled in the underlying webserver
code. This patch introduces WebResponse and PrerenderedWebResponse
structs that wrap and replace the 'output' EasyJson and ostringstream
pointers, respectively, used before, and which has fields for the
response code and additional headers.

The ability to add headers isn't currently used, but it's nice to have.
The response codes are adjusted where necessary to match what one
would expect, e.g. navigating to /tablet?id=foo returns
404.

Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
---
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-path-handlers.h
M src/kudu/server/default-path-handlers.cc
M src/kudu/server/pprof-path-handlers.cc
M src/kudu/server/rpcz-path-handler.cc
M src/kudu/server/tracing-path-handlers.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/thread.cc
M src/kudu/util/web_callback_registry.h
12 files changed, 226 insertions(+), 109 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/1
--
To view, visit http://gerrit.cloudera.org:8080/8141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924
Gerrit-Change-Number: 8141
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley