Community
Participate
Working Groups
Seen while running the org.eclipse.rse.tests.subsystems.files/FileServiceArchiveTest : The Archive Handlers, and ArchiveHandlerManager print lots of messages to stdout. This should not happen in an Eclipse based product -- Instead it should either throw proper exceptions, or use logging facilities like it is done in other parts of RSE. In some cases, the logged messages are really confusing, e.g. FileServiceArchiveTest.testCreateZipFile() LocalFileService.createFile() ArchiveHandlerManager.getInstance().createEmptyArchive() prints Could not open zipfile: ...\rsetest5373557139\dummy.zip error in opening zip file true true even if everything works exactly as expected, and the archive is successfully created. Seeing an error message in case of successful completion is a no-no, just like printing "true" which has no meaning or information for the user. I think that such false messages should be eliminated completely, and all error messages from the archive handlers should be logged rather than printed to stdout. Logging is available even in the dstore server, as per bug 193426 comment 5.
Right now, client and server part use different logging implementations. For client, SystemBasePlugin has several log methods, which will direct the information to org.eclipse.rse.logging.Logger, which in turn get: InternalPlatform.getDefault().getLog(bundle); For server, UniversalServerUtilities (which is shipped in universal.miner) provides the log methods, which in turn direct its log calls to ServerLogger. ServerLogger also shipped in universal.miner. Since Archive handling code could be invoked from client or server, it should not assume which log is available. So I think we need to define a generic logging machemism in the client_server package to direct the logging into available one logging support, probably using Java Reflection API.
I think there are two separate issues which we should not confuse: 1. Normal operation - optionally with (1a) logging 2. Error conditions Let's deal with (2) first - error conditions: --------------------------------------------- Most of the System.out.println() in archive handlers are dealing with error conditions, i.e. they catch an IOException, print the exception to stdout, and return "false". Even in the ISystemArchiveHandler interface, most methods are declared to return a boolean to indicate successful completion or error. The client of the archive handler will only know that the planned operation failed, but will not get any further information what failed, and why. If we had a pluggable logging mechanism for client and server, we could log those messages but I don't think it would help the actual client too much, who just received information that an operation failed; especially remote users would not know where to look up the server log to understand what failed exactly. Local users might find the Eclipse Error Log, but perhaps not too easily. So for now, printing errors to stdout is just as good as any other logging facility -- for the future, we should improve the error reporting, and this will definitely require an API change. I filed bug #199854 for this. Now for (1) - normal operation: ------------------------------- I don't think that anything needs to be logged during normal operation. Still there is code like in SystemZipHandler#replaceOldZip(): System.out.println(_file.renameTo(oldFile)); System.out.println(outputTempFile.renameTo(_file)); This will print "true" "true" even in case of perfectly normal operation. This implementation should be changed to only print in case of error conditions. That's what I'd like to use this current bug report for -- I've changed the summary accordingly. Finally for (1a) - logging: --------------------------- Even during normal operation, it may be helpful to log certain events for debugging purposes; or, while the new error handler API from (2) is not yet in place, error conditions can be sent to a proper log. A nice description of what can or should be logged, and different logging use cases, is at http://java.sun.com/j2se/1.4.2/docs/api/java/util/logging/package-summary.html#package_description I have filed bug 199858 comment 2 asking for proper logging in the archive handlers, and I'm proposing to use the java.util.logging facility for this. Again - regarding this current bug - I'd consider it fixed when the archive handlers don't print any more messages during normal operation (which is exposed through the unit tests). For error reporting and logging, the other two bugs have been filed.
Bulk update target milestone 2.0.1 -> 3.0
