surahman commented on pull request #3786: URL: https://github.com/apache/incubator-heron/pull/3786#issuecomment-1058271543
> But looking back at old versions of the code, this UI query may never have matched up with the tracker api. Very odd. My guess is technical debt. > I personally prefer the `/metrics/query` syntax, but might be easier to just roll back to `/metricsquery` style route. I agree. Good REST API design necessitates versioning of the API as well as nesting under path routes. All metrics queries should be nested under the `/metrics/` path. I have never used Node.js and I am not sure if the URL needs to have the `?` embedded in it - I think the framework should deal with the query string delimiter itself. We typically do not have to do this in libraries for C++/Java/Golang. https://github.com/apache/incubator-heron/blob/dd23309cba06853e495f378f5d2cf2c50706a41e/heron/tools/ui/resources/static/js/plan-stats.js#L378 Please ping when ready for review. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
