I have uploaded a new version of the patch addressing some of the criticism that was presented. Can someone please take a look at it once more?
Code review: http://codereview.appspot.com/10269/show JIRA: https://issues.apache.org/jira/browse/SHINDIG-775 On Wed, Dec 17, 2008 at 12:56 PM, Lev Epshteyn <[email protected]> wrote: > First, let me answer why I didn't want to use jetty's built-in static > serving capabilities. The Idea here is to enable sample container to be used > as a debug environment. To that end, I think it's a huge value-add to enable > people to render static gadget xml files from their filesystem without > running an additional server just to serve them up. > > My goal is to create a zero-configuration distribution to allow gadget > developers (who don't necessarily know anything about java, servlets, > shindig and jetty) to quickly iterate on their gadgets without uploading > them to a remote server and installing them in sandboxes or production > containers. > > Since there is no way to guess where on the filesystem the gadget XML specs > will be sitting, I wish to enable this dev server to have access to the > entire filesystem. The alternatives would be requiring the developer to > configure jetty's static file serving themselves, or else asking them to > copy their gadget spec files into some well-known location. Both of these > put up barriers to zero-hassle development, and I decided not to go that > route. > > Let me know if given these reasons you still think it's a bad idea. > > As far as the location of the code, I am more than willing to move it into > a more appropriate package - please let me know the specific location you > have in mind. Per people's suggestions, I have already commented out the > servlet declaration/mappings from web.xml files, and put a comment into the > java source that this is only to be used for debugging. I can also add a log > entry on startup, if you think that will keep people from erroneously > enabling it in production environment. > > (I will likely not be able to make these changes until the New Year, as I > will be traveling starting today.) > > > On Wed, Dec 17, 2008 at 12:31 PM, <[email protected]> wrote: > >> First and most important question for me would be: What do you need this >> for? As Kevin points out, let Jetty serve the static files. >> >> If this is a debugging aid for the sample container, the code should not >> be in the gadgets module. It belongs in the samples or server module >> (maybe server will sprout its own java code base for pure sample >> purposes). >> >> This thing as a tremendous potential to be abused, if it goes in, I >> expect a lot of patch suggestions to remove the localhost test. And it >> breaks if I try to access a file using my regular host name (which I do >> all the time), because it resolves to my network Ip and not localhost. >> >> Summary: Don't put this in. There are better ways to do it. If you >> insist to put it in, please don't put it into the gadgets codebase, put >> it into the server or samples and clearly mark it as "DEBUG CODE. DO NOT >> RUN IN PRODUCTION" (e.g. log this on startup. >> >> >> >> http://codereview.appspot.com/10269 >> > >
