Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15292 )
Change subject: [webserver] add information on time source ...................................................................... Patch Set 2: (14 comments) http://gerrit.cloudera.org:8080/#/c/15292/2//COMMIT_MSG Commit Message: PS2: > I think those extra knobs _could_ make sense in the general case, but speci Thank you for the feedback! All right, let's then keep this approach for the '/config' page. http://gerrit.cloudera.org:8080/#/c/15292/2//COMMIT_MSG@9 PS2, Line 9: This patch adds the following information into the '/config' endpoint > Would love to see a screenshot or two showing the new page. Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc File src/kudu/server/default_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@293 PS2, Line 293: static void FillSecurityConfigs(EasyJson* output) { > Apart from the kFoo consolidation, has this changed at all? Right: nothing else changed but kFoo consolidation & 'explanation' --> 'comment' renaming. I'm not sure why it's shown out-of-nowhere way in the gerrit's diff, probably because the context of ConfigurationHandler() function has changed. http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@344 PS2, Line 344: static const char* const kName = "name"; : static const char* const kValue = "value"; : static const char* const kId = "id"; : static const char* const kComment = "comment"; > These need to be the same as in FillSecurityConfigs() for the mustache stuf Yes, these and those in the corresponding mustache file should be in sync to make it work. Done. http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@373 PS2, Line 373: best > the best Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@375 PS2, Line 375: In case of other settings > Nit: Otherwise, Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@375 PS2, Line 375: setting > Nit: drop this Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@376 PS2, Line 376: always > Nit: drop this. Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@387 PS2, Line 387: Servers > Nit: NTP Servers Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@392 PS2, Line 392: the --builtin_ntp_servers flag > Nit: just --builtin_ntp_servers, don't need 'the' or 'flag'. Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@392 PS2, Line 392: In case > Nit: don't need Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@395 PS2, Line 395: hosting > the hosting Done http://gerrit.cloudera.org:8080/#/c/15292/2/src/kudu/server/default_path_handlers.cc@396 PS2, Line 396: the --builtin_ntp_servers flag > Nit: don't need 'the' or 'flag'. Done http://gerrit.cloudera.org:8080/#/c/15292/2/www/config.mustache File www/config.mustache: http://gerrit.cloudera.org:8080/#/c/15292/2/www/config.mustache@44 PS2, Line 44: {{/security_configs}} > This seems out of place? Whoops, indeed. Interesting, but it was resilient enough to display the page. Fixed. -- To view, visit http://gerrit.cloudera.org:8080/15292 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ff07808a1a534f3546e88ff13bc876b98afdb7e Gerrit-Change-Number: 15292 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin <[email protected]> Gerrit-Comment-Date: Wed, 26 Feb 2020 03:42:18 +0000 Gerrit-HasComments: Yes
