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 <wdberke...@gmail.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Sat, 30 Sep 2017 00:18:23 +0000 Gerrit-HasComments: Yes
