http://gwt-code-reviews.appspot.com/130806/diff/1/2 File dev/core/src/com/google/gwt/core/ext/ServletContainerLauncher.java (right):
http://gwt-code-reviews.appspot.com/130806/diff/1/2#newcode70 Line 70: public boolean processArguments(TreeLogger logger, String arguments) { Another way to do this would be to introduce an additional start method that has the "arguments" argument. http://gwt-code-reviews.appspot.com/130806/diff/1/2#newcode97 Line 97: public void setNormalRequestLogLevel(Type logLevel) { On 2010/01/14 16:59:28, jat wrote:
On 2010/01/14 16:50:28, tobyr wrote: > Why is this only for "non-failing" web requests? In general I'm not
sure how
> this is supposed to integrate with other logging systems that are
much more
> sophisticated. For example, in App Engine, which uses
java.util.logging
> configuration, you can control the logging levels of each logger
individually.
This is GPE request -- they don't want normal request showing up in
the logs by
default, so previously there was an instanceof hack to call a special method on JettyLauncher to set
it. If we
are changing the API, it seemed prudent to replace the hack.
In particular, this refers to the log level of messages sent to the
logger
supplied in the start method.
If you think this shouldn't be there, we can leave the hack for now.
John is correct; we did request this. Toby, you're right that this solution does not integrate well with other logging solutions. However, this is a general problem with the way in which the JettyLauncher SCL was written - it re-routes Jetty's RequestLog messages to the TreeLogger, regardless of the log level that the the requestlog messages were actually logged at - it only pays attention to the response codes (500, 404, 200, etc..). If we want to fix this, we need to do some digging to see how Jetty's RequestLog ties in with other logging frameworks, and then have its messages logged to the TreeLogger based on their log level, as opposed to their response codes. While I agree that we need to figure out if there's a better way that we can handle this, I don't think it should hold up this patch. I'd leave the hack in as-is for now, and we can discuss this issue after this patch lands. http://gwt-code-reviews.appspot.com/130806/diff/1/6 File dev/core/src/com/google/gwt/dev/DevMode.java (right): http://gwt-code-reviews.appspot.com/130806/diff/1/6#newcode84 Line 84: return new String[] {"servletContainerLauncher[:args]"}; What if the args have spaces in them? http://gwt-code-reviews.appspot.com/130806/diff/1/6#newcode420 Line 420: ui.setCallback(RestartServerEvent.getType(), null); Why is this necessary? Won't this cause the termination of the DevMode process? http://gwt-code-reviews.appspot.com/130806/diff/1/5 File dev/core/src/com/google/gwt/dev/DevModeBase.java (right): http://gwt-code-reviews.appspot.com/130806/diff/1/5#newcode152 Line 152: // this isn't fully accurate, as their is no guarantee we will get nit: their --> there http://gwt-code-reviews.appspot.com/130806/diff/1/5#newcode566 Line 566: protected interface OptionBindAddress { You should doc the difference between the bind address and the connect address, and when each is used in the DevMode implementation. http://gwt-code-reviews.appspot.com/130806/diff/1/3 File dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java (left): http://gwt-code-reviews.appspot.com/130806/diff/1/3#oldcode498 Line 498: public void setBaseLogLevel(TreeLogger.Type baseLogLevel) { We may want to leave this hack in until we decide what to do about the logging issue. That way, we won't hold up this patch. But, now seems the right time to discuss this issue. http://gwt-code-reviews.appspot.com/130806/diff/1/3 File dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java (right): http://gwt-code-reviews.appspot.com/130806/diff/1/3#newcode517 Line 517: if (bindAddress != null) { If bindAddress is null, how do you get the server's IP back out to the dev mode query params? I guess you assume that it is 127.0.0.1? http://gwt-code-reviews.appspot.com/130806
-- http://groups.google.com/group/Google-Web-Toolkit-Contributors
