----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62781/#review187294 -----------------------------------------------------------
Fix it, then Ship it! Thanks for taking this on! Just a few minor comments and we can get this committed. src/webui/master/static/js/controllers.js Lines 30-41 (patched) <https://reviews.apache.org/r/62781/#comment264219> Some documentation would be great, not sure people will get what 'includeId' is for. E.g. ``` // Returns the URL prefix for an agent, there are two cases // to consider: // // (1) Some endpoints for the agent process itself require // the agent PID.name in the path, this is to ensure // that the webui works correctly when running against // mesos-local or other instances of multiple agents // running within the same host:port: // //hostname:port/slave(1) // //hostname:port/slave(2) // ... // // (2) Some endpoints for other components in the agent // do not require the agent PID.name in the path, since // a single endpoint serves multiple agents withint the // same process. In this case we just return: // //hostname:port // // Note that there are some clashing issues in mesos-local // (e.g. hosting '/slave/log' for each slave log, we don't // namespace metrics within '/metrics/snapshot', etc). ``` src/webui/master/static/js/controllers.js Lines 30-31 (original), 43-44 (patched) <https://reviews.apache.org/r/62781/#comment264220> Stale? src/webui/master/static/js/controllers.js Line 1096 (original), 1100 (patched) <https://reviews.apache.org/r/62781/#comment264225> Let's say we set false here because this is used for generating /files links? - Benjamin Mahler On Oct. 6, 2017, 6:42 p.m., Gaston Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62781/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2017, 6:42 p.m.) > > > Review request for mesos, Benjamin Mahler and James Peach. > > > Repository: mesos > > > Description > ------- > > This path extracts the generation of URLs of agent endpoints to a single > function. > > Having this logic in just one place makes it easier to modify it, e.g., > to make it possible to use the UI via a reverse proxy. > > > Diffs > ----- > > src/webui/master/static/browse.html > c586b9050e14697ca89fabfe636c2d18a4ecf6f5 > src/webui/master/static/js/controllers.js > cb1f093ea21864eadd0ae32f3c33a7ff88b6fc27 > src/webui/master/static/js/services.js > b99ad685e9614ef77586efb5fa52345fc173e51e > > > Diff: https://reviews.apache.org/r/62781/diff/4/ > > > Testing > ------- > > Verified that the UI still works when running a master and an agent. > > I tried: > > * Clicking all the tabs. > * Viewing the master/agent logs. > * Browsing a task sandbox and reading a file using the pailer. > > > Thanks, > > Gaston Kleiman > >
