Hi Kevin, Thank you for your valuable feedback about this. I'll fix these and try to find out solution for eclipse licensing issue.
Thanks, Milinda On Wed, Dec 2, 2009 at 5:46 AM, Kevin Sutter (JIRA) <[email protected]> wrote: > > [ > https://issues.apache.org/jira/browse/OPENJPA-1403?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12784556#action_12784556] > > Kevin Sutter commented on OPENJPA-1403: > --------------------------------------- > > Hi Milinda, > I haven't fully absorbed your patch and its implications yet, but I do have > some initial comments on the patch content and format... > > o Classloading is a touchy area. OpenJPA needs to work in a standalone > Java SE environment, an application server Java EE environment, and now the > OSGi bundled environment. As we integrated with the various application > servers, we had to ensure that the classloading mechanisms continued to work > in these other environments. I'm not sure how much verification you have > done with your proposed patch in these other environments. On first glance, > I didn't see that this alternate classloading mechanism kicked in just for > the OSGi environment, or whether you are proposing that this new mechanism > replaces our current classloading mechanism. Just makes me nervous when I > see changes to classloading... > > o None of the new files have the Apache license at the top of the file. > > o I saw this author tag in one of the files that makes me very nervous... > > + * @author shsmith from EclipseLink > > You mentioned that you based this patch off of how EclipseLink did > something. We can't just take code from EclipseLink without providing > proper credit. To be honest, I'm not sure how the licensing would work in > this case. Was this code covered by the Eclipse Public License? Need more > investigation and justification before we could ever accept code like this. > > o Using splat (*) in the import statements is not sufficient. With the > tools available today, you can easily clean this up with the explicit > imports. > > o Test cases? Not only for the OSGi environment, but if you are affecting > current classloading, then what assurances do we have that nothing was > affected? > > Trying hard not to rain on your parade. Allowing OpenJPA to play in the > OSGi environment is important to the project and we appreciate the input. > We just have a large set of customers that we need to continue to support. > > Thanks, > Kevin > > > OSGi Aware Persistence Provider Implementation > > ---------------------------------------------- > > > > Key: OPENJPA-1403 > > URL: https://issues.apache.org/jira/browse/OPENJPA-1403 > > Project: OpenJPA > > Issue Type: New Feature > > Reporter: Milinda Lakmal Pathirage > > Attachments: osgi.patch > > > > > > Current OpenJPA trunk implementation doesn't have full support for using > OpenJPA in OSGi containers. For example when OSGi bundle which use OpenJPA > deployed, OpenJPA persistence provider cannot locate the persistence.xml in > that bundle due to class loading differences in OSGi environment. > EclipseLink has resolved this by using bundle listeners and JPA specific > OSGi bundle header. Patch provided in this JIRA solve issues in OpenJPA in > OSGi environment by following method used in EclipseLink. But there is a > problem with current OpenJPA implementation which caused me to add > Dynamic-Imports header to OpenJPA OSGi bundle to allow loading classes from > bundles that use OpenJPA. I think current OpenJPA implementation doesn't > provide support to replace central class load to support loading classes > from bundles which use OpenJPA. If we have that support we'll be able to > remove Dynamic-Imports and make OpenJPA OSGi bundle follow OSGi best > practices . > > Please review the path and provide your ideas about this patch. > > -- > This message is automatically generated by JIRA. > - > You can reply to this email to add a comment to the issue online. > > -- Milinda Pathirage Senior Software Engineer & Product Manager WSO2 BPS; http://wso2.org/bps WSO2 Inc.; http://wso2.com E-mail: [email protected], [email protected] Web: http://mpathirage.com Blog: http://blog.mpathirage.com
