Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14573 )
Change subject: webserver: add support for Knox URL rewriting ...................................................................... Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/14573/6/src/kudu/integration-tests/webserver-crawl-itest.cc File src/kudu/integration-tests/webserver-crawl-itest.cc: http://gerrit.cloudera.org:8080/#/c/14573/6/src/kudu/integration-tests/webserver-crawl-itest.cc@235 PS6, Line 235: link = StringReplace(link, kKnoxPrefix, "", /* replace_all= */ false); > To clarify, misapplied encoding (i.e. encoding when we shouldn't have) won' Ok. Having Knox serve broken links and breaking Knox feel the same to me from Kudu's POV. http://gerrit.cloudera.org:8080/#/c/14573/6/src/kudu/integration-tests/webserver-crawl-itest.cc@314 PS6, Line 314: vector<string> sorted_urls(urls_seen.begin(), urls_seen.end()); : std::sort(sorted_urls.begin(), sorted_urls.end()); : LOG(INFO) << "Dumping visited URLs"; : for (const auto& u : sorted_urls) { : LOG(INFO) << u; : } : } > I'm tempted to say no because of the additional effort required to collect It doesn't really. I'm suggesting testing that we've actually visited a page. For instance, if for whatever reason the web UI is broken in such a way that no links are generated, this test would still pass. -- To view, visit http://gerrit.cloudera.org:8080/14573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee92cb094b81609356acf858b7c549b6c281a7e5 Gerrit-Change-Number: 14573 Gerrit-PatchSet: 7 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 02 Nov 2019 05:37:19 +0000 Gerrit-HasComments: Yes
