EclipseLink: allow reading persistence.xml from the filesystem#613
EclipseLink: allow reading persistence.xml from the filesystem#613eric-maynard merged 1 commit intoapache:mainfrom
Conversation
| Arguments.of(confJar + "!/persistence.xml", true), | ||
| Arguments.of(confJar + "!/dummy.xml", false), |
There was a problem hiding this comment.
To make sure I'm reading these correctly, this means the new code should work with existing config + jar setups?
There was a problem hiding this comment.
Yes, it's my understanding reading the code: it's "backward" compatible, fallbacking to the "inner" persistence xml.
There was a problem hiding this comment.
Yes, it's backwards-compatible.
There was a problem hiding this comment.
Could it be made at build time?.. or perhaps by test code itself?
There was a problem hiding this comment.
Certainly... it's just 1.5Kb though, is that worth the hassle?
There was a problem hiding this comment.
My concern is that if EclipseLink config evolves, the jar would go out of sync with the production code... But this is not likely, I guess... I think it's ok to merge "as is",
There was a problem hiding this comment.
I do think in the future we should be making more stuff at run/test time (e.g. the python client code can be generated from the spec). But I agree that doesn't need to block this fix
|
Related: #95. |
I turns out it is actually possible to read a persistence.xml file directly from the filesystem.
The trick is that it must be in a directory that will be considered as the "archive directory"; the
eclipselink.persistencexmlproperty must point to the path of the file relative to the archive directory's parent. E.g. if the persistence file is in/config/persistence.xml, the archive directory is/configand theeclipselink.persistencexmlproperty must beconfig/persistence.xml.This PR builds on that observation and expands the allowed locations for a persistence.xml file to the following ones:
Note: this PR does not modify the Helm chart, but we could look into simplifying it to use plain files instead of jars.