> On 2010-08-18 12:50:51, Philip Zeyliger wrote: > > lang/java/src/java/org/apache/avro/ipc/trace/TraceClientServlet.java, line > > 170 > > <http://review.cloudera.org/r/655/diff/2/?file=7322#file7322line170> > > > > This seems very ad hoc. What you're looking for is some sort of URL > > routing. You have a couple of options here: > > > > - Servlets have something called a RequestDispatcher which can be > > controlled by a "web.xml" file. > > - Or do it a bit more methodically, by extracting the path component > > out of the URL (instead of stripping http://, which is one-off, and may be > > broken because we might be using https), and then having some logic. > > > > It makes sense to put the logic for serving a specific view into its > > own function. If you had a more heavy-weight framework, it would certainly > > force you to do it. Either way, separating out the URL parsing logic from > > the collection logic seems like a nice thing to do.
Yep, agree it makes sense do modularize the view functions. I will probably opt for method (2) because tooling around with the Servlet stuff always confuses me. If we end up expanding to a ton of views we can always plug that in afterwards. - Patrick ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/655/#review945 ----------------------------------------------------------- On 2010-08-16 16:10:06, Patrick Wendell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/655/ > ----------------------------------------------------------- > > (Updated 2010-08-16 16:10:06) > > > Review request for Avro and Philip Zeyliger. > > > Summary > ------- > > Basic collection of multiple traces and viewing servlet. Note: the javascript > files are external files, not mine. > > > Diffs > ----- > > lang/java/src/java/org/apache/avro/ipc/stats/StaticServlet.java 65a3cbe > lang/java/src/java/org/apache/avro/ipc/trace/SpanAggregator.java 65e4690 > lang/java/src/java/org/apache/avro/ipc/trace/StaticServlet.java > PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/Trace.java 46a2fce > lang/java/src/java/org/apache/avro/ipc/trace/TraceClientServlet.java > 016d4d4 > lang/java/src/java/org/apache/avro/ipc/trace/TraceCollection.java > PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/TraceNode.java 5d62aa2 > lang/java/src/java/org/apache/avro/ipc/trace/TracePlugin.java c63ca5b > lang/java/src/java/org/apache/avro/ipc/trace/TracePluginConfiguration.java > bc1aaee > lang/java/src/java/org/apache/avro/ipc/trace/Util.java 01c9e9e > lang/java/src/java/org/apache/avro/ipc/trace/static/g.bar.js PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/static/jquery.tipsy.js > PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/static/protovis-r3.2.js > PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/static/tipsy.js PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/templates/collection.vm > PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/templates/common.vm > PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/templates/overview.vm > PRE-CREATION > lang/java/src/java/org/apache/avro/ipc/trace/templates/traceinput.vm > PRE-CREATION > lang/java/src/test/java/org/apache/avro/ipc/trace/TestBasicTracing.java > 6bc9153 > lang/java/src/test/java/org/apache/avro/ipc/trace/TestTraceCollection.java > PRE-CREATION > share/schemas/org/apache/avro/ipc/trace/avroTrace.avpr 6cf25d2 > > Diff: http://review.cloudera.org/r/655/diff > > > Testing > ------- > > TestTraceCollection.java tests the new TraceCollection class. > > > Thanks, > > Patrick > >
