Re: WASTransformer
On Oct 27, 2006, at 9:41 AM, Abe White wrote: Does anyone mind if I move this class from the org.apache.openjpa.util package to the org.apache.openjpa.ee package? It's a very EE-specific class, and in my mind is not a general utility other parts of the system will ever use. I'd even consider removing the class altogether and just moving its main() method to the WASManagedRuntime -- it's really just a build helper for the WASManagedRuntime. +1 for moving it to org.apache.openjpa.ee. Alternatively, we could establish some sort of top-level build module and put it there, as the class is only invoked as part of the build process. Or even do the work of the class in beanshell or something in our build scripts (assuming maven supports it). Ideally, it would be a Maven plugin, but unfortunately, I haven't found any simple way of including a plugin with the OpenJPA source. The Maven plugin API is simple enough to write to, but Maven seems to require that the plugin be installed in the repository in order to be usable by a project, which means that it can't just be included as a module to the openjpa project, but instead needs to be maintained as a separate project. Very annoying. Anyway, I don't think it is too bad to invoke it from the maven- antrun-plugin. It is possible to embed a beanshell script within a maven-antrun-plugin as well, but if the logic is at all complex, it is just as easy to include it in a helper class. I have no opinion on whether the logic should be moved into WASManagedRuntime.main() or not. Thoughts? __ _ Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: WASTransformer
+1 for moving to org.apache.openjpa.ee. I don't really have a strong feeling about where the class should go. As you said it's really a build utility, I just wasn't sure where the best place was to put it. +0.5 for moving the logic in to WASManagedRuntime.main(). Go ahead and move it unless someone objects, there's no real need for another class. +0 on beanshell. I don't know enough about beanshell, I don't have a reason to not change it if beanshell will work better. -Mike On 10/27/06, Marc Prud'hommeaux [EMAIL PROTECTED] wrote: On Oct 27, 2006, at 9:41 AM, Abe White wrote: Does anyone mind if I move this class from the org.apache.openjpa.util package to the org.apache.openjpa.ee package? It's a very EE-specific class, and in my mind is not a general utility other parts of the system will ever use. I'd even consider removing the class altogether and just moving its main() method to the WASManagedRuntime -- it's really just a build helper for the WASManagedRuntime. +1 for moving it to org.apache.openjpa.ee. Alternatively, we could establish some sort of top-level build module and put it there, as the class is only invoked as part of the build process. Or even do the work of the class in beanshell or something in our build scripts (assuming maven supports it). Ideally, it would be a Maven plugin, but unfortunately, I haven't found any simple way of including a plugin with the OpenJPA source. The Maven plugin API is simple enough to write to, but Maven seems to require that the plugin be installed in the repository in order to be usable by a project, which means that it can't just be included as a module to the openjpa project, but instead needs to be maintained as a separate project. Very annoying. Anyway, I don't think it is too bad to invoke it from the maven- antrun-plugin. It is possible to embed a beanshell script within a maven-antrun-plugin as well, but if the logic is at all complex, it is just as easy to include it in a helper class. I have no opinion on whether the logic should be moved into WASManagedRuntime.main() or not. Thoughts? __ _ Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: WASTransformer
+0.5 for moving the logic in to WASManagedRuntime.main(). Go ahead and move it unless someone objects, there's no real need for another class. I went ahead and did this. I also moved WASManagedRuntime's caching logic to its endConfiguration() callback to avoid the threading issues that seemed inherent in using delayed caching. Finally, I made the members of WASManagedRuntime private instead of protected, just because the rest of the codebase only uses protected members when a class is meant to be extended. Let me know if you see problems with any of these changes. ___ Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: WASTransformer
+0.5 for moving the logic in to WASManagedRuntime.main(). Go ahead and move it unless someone objects, there's no real need for another class. I went ahead and did this. I also moved WASManagedRuntime's caching logic to its endConfiguration() callback to avoid the threading issues that seemed inherent in using delayed caching. Finally, I made the members of WASManagedRuntime private instead of protected, just because the rest of the codebase only uses protected members when a class is meant to be extended. Let me know if you see problems with any of these changes. One more thing: I removed the logging of errors that are thrown as exceptions. IMO, if you're throwing an exception, you're making it the caller's choice on how to handle it, including whether to log it. That's been our practice throughout the codebase, anyway. Otherwise you tend to wind up with the same errors being recorded many times. ___ Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: WASTransformer
On 10/27/06, Abe White [EMAIL PROTECTED] wrote: +0.5 for moving the logic in to WASManagedRuntime.main(). Go ahead and move it unless someone objects, there's no real need for another class. I went ahead and did this. I also moved WASManagedRuntime's caching logic to its endConfiguration() callback to avoid the threading issues that seemed inherent in using delayed caching. Finally, I made the members of WASManagedRuntime private instead of protected, just because the rest of the codebase only uses protected members when a class is meant to be extended. Let me know if you see problems with any of these changes. One more thing: I removed the logging of errors that are thrown as exceptions. IMO, if you're throwing an exception, you're making it the caller's choice on how to handle it, including whether to log it. That's been our practice throughout the codebase, anyway. Otherwise you tend to wind up with the same errors being recorded many times. I screwed up there. I should have used log.Trace() instead of log.Error(). I agree that we shouldn't be logging exceptions as a matter of course (certainly not at the most visible logging level). In general I think it is useful to include them in diagnostic traces. I usually direct trace to a file and I've found it helpful to have the exceptions included in the trace (maybe that's because I'm lazy and I don't handle the exceptions as well as I should in the app). On the other hand it looks like the EntityManager exposes the configuration and the log to the application and the application could log the message. If our best practice is not to write to the log when you throw an exception then I'll avoid doing it in the future. Thanks for catching it for me. ___ Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it. -- -Michael Dick
