-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Review and promote Jakarta Persistence TCK for JakartaEE 9 release #385
Comments
@scottmarlow what exactly I'm supposed to confirm?? This is not assigned to me. I have also never been doing any changes to TCKs nor I'm planning to do directly. I also do not have enough rights to work on TCK project/jobs themselves. |
Have you reviewed the archive (.zip) contents for the TCK? Are you
satisfied with the the contents? This is part of the Specification so
they need to tell the TCK dev. team that the contents meet the
requirements of the API committer team.
I think the TCK documentation is updated correctly. If you (speaking for
the API committer team for Jakarta Persistence) are satisfied with the
contents you can approve the TCK. If you think others on the API team
need to review, you can ask for that. Once that's all done, just reply
that this is ready to be promoted as a Stable release and the TCK team
will stop building it -- unless there's some explicit directive or
request to build it again.
…On 8/10/2020 12:30 PM, Lukas Jungmann wrote:
@scottmarlow <https://github.com/scottmarlow> what exactly I'm
supposed to confirm?? This is not assigned to me. I have also never
been doing any changes to TCKs nor I'm planning to. I also do not have
enough rights to work on TCK project/jobs themselves.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#385 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC5WM3V5Y7PI43PMGCBQUGTSABDHDANCNFSM4PI4KDHA>.
|
OK, where can I find the zip I'm supposed to review if it is not the one under https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-900/ folder? The one here has ie wrong jpa spec version number, among other things. |
it should be
https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-900/jakarta-persistence-tck-3.0.0.zip
Anything incorrect, please list here.
…-- Ed
On 8/10/2020 1:17 PM, Lukas Jungmann wrote:
Have you reviewed the archive (.zip) contents for the TCK?
OK, where can I find the zip I'm supposed to review if it is not the
one under
https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-900/
folder? The one here has ie wrong jpa spec version number, among other
things.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#385 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC5WM3VNAW2DZIQNSKTKMKTSABITZANCNFSM4PI4KDHA>.
|
basically #364 is not fixed, that is currently the main blocker. |
Where does this appear in the documentation?
…On 8/10/2020 1:57 PM, Lukas Jungmann wrote:
basically #364
<#364> is not
fixed, that is currently the main blocker.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#385 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC5WM3XSLUQNTAZX2GMJ4NTSABNMLANCNFSM4PI4KDHA>.
|
I wasn't aware these properties (or the results of their settings) showed up in any of the material included in the TCK zip files. However, I have proposed #419 to update the user_guide pom file and also the _config.yml file. Please check th PR and let me know if more changes are needed. |
Just FYI, after the documentation changes for persistence was made in #411 (and also #419) , it would require to regenerate the tck bundles to be updated in https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-900/jakarta-persistence-tck-3.0.0.zip. |
I have merged #419 . Please let us know if it is ok to refresh the https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-900/jakarta-persistence-tck-3.0.0.zip . |
Is there a nightly where I can verify recent changes? The most recent build I found at https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9/nightly/ is from July 30. Based on PRs and briefly checking sources, it looks good to me |
… On 8/11/2020 1:02 PM, Lukas Jungmann wrote:
Is there a nightly where I can verify recent changes? The most recent
build I found at
https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9/nightly/
<https://urldefense.com/v3/__https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9/nightly/__;!!GqivPVa7Brio!L1lAFXhicki6CgH8R0ryFvJOvgwggdW5M7cXfCBLYWDqy5GQkt5qCHN3mUR0qdI$>
is from July 30. Based on PRs and briefly checking sources, it looks
good to me
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://urldefense.com/v3/__https://github.com/eclipse-ee4j/jakartaee-tck/issues/385*issuecomment-672249355__;Iw!!GqivPVa7Brio!L1lAFXhicki6CgH8R0ryFvJOvgwggdW5M7cXfCBLYWDqy5GQkt5qCHN3-Dmkl0U$>,
or unsubscribe
<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AC5WM3S7NPT5THZU3ZBME5TSAGPW3ANCNFSM4PI4KDHA__;!!GqivPVa7Brio!L1lAFXhicki6CgH8R0ryFvJOvgwggdW5M7cXfCBLYWDqy5GQkt5qCHN3sakHlmQ$>.
|
based on the current staged build:
|
Thanks, I merged this and refreshed the bundle.
@gurunrao @scottmarlow Do you know if we need to update the schemas with the https://jakarta.ee/xml/ns/persistence for jpa |
@alwin-joseph if we update the persistence tests to use https://jakarta.ee/xml/ns/persistence, that would likely mean that JPA 1.0 persistence providers would not work with Jakarta EE 9. Some years ago, the JPA TCK used to include a TopLink (JPA 1.0 I think) persistence provider that was used to test that EE servers still supported JPA 1.0 persistence providers. This was rewritten and no longer tests with a JPA 1.0 persistence provider, however I think the contract for JPA 1.0 support is still required by the https://jakarta.ee/specifications/persistence/3.0/persistence_3.0-RC2.html#a12802 which mentions:
In contrast, Jakarta EE 9 is only required to support backward compatibility with Jakarta EE 8 compatible applications. IMO, JPA has its own rules, as mentioned in https://jakarta.ee/specifications/persistence/3.0/persistence_3.0-RC2.html#a12802. I do speak from my experience developing + maintaining the WildFly/JBOSS JPA container (from JBoss AS 7+) and being on the JPA expert group (I think from 2.0+). @lukasj do you agree^? In theory, we could add/change a single JPA test to use the https://jakarta.ee/xml/ns/persistence/ to ensure that all tested persistence providers (including EclipseLink) support https://jakarta.ee/xml/ns/persistence but I think we are past the point of adding new tests. |
wrt persistence spec itself - given the requirement on backward compatibility is EE 8 only, I'd just remove versions < 2.2 from the spec and add 3.0 there. I believe it's better to be done now than later (which can easily lead to never).
Wouldn't the same be true even for 2.0 - generally for anything "older" than 3.0?. I also wouldn't be surprised if these new namespaces are missed on the implementations side... |
@lukasj Yes, you are right, as anything older than 3.0 will still reference javax class namespace. So, yes, we should update to https://jakarta.ee/xml/ns/persistence. cc: @alwin-joseph |
The reason being that JPA 3.0 persistence providers shouldn't be required to support the older schemas. |
I also expect that many JPA container + Persistence providers will also still handle the older schemas for support of the linked https://jakarta.ee/specifications/persistence/3.0/persistence_3.0-RC2.html#a12802 section. |
@edbratt few minor things:
the only remaining item is how to deal with updated schema... |
How? I think it depends on if we agree on the reason for switching persistence.xml/orm.xml to 3.0 persistence XSD, being We can change one persistence.xml or all of them, there aren't that many. Same with orm.xml:
The pluggability tests could also be updated to 3.0. |
the decision here is going to have cascading effect on other specs as there are other schemas as well, ie soap ws do have bunch of them, then there are also ie faces, bean val.... It does not make much sense to me for one spec to have different handling and tests than others. If the decision is that new namespaces are optional - not what I would prefer but I could live with it - then we're already done. Anything else means some work to do and the question is how much work that would be. |
Good points @lukasj, thanks for raising them and clarifying. I will step back and restate your question now but first, there was a related vote on the https://www.eclipse.org/lists/jakartaee-platform-dev/msg01400.html thread, I do see various votes for #3 that is reflected in the Jakarta EE Platform call minutes from January 28, 2020. So, in summary, the question is are we required to update TCK tests to use the EE 8+ schemas? What would the impact be if we don't complete update the remaining TCK references to schemas that pre-date Jakarta EE 8? What would the impact be if we do update the remaining TCK test references to schemas that pre-date Jakarta EE 8? For reference the issue for updating the remaining schemas is #266 (I updated a comment at the end to include an updated list of http://xmlns.jcp.org/xml/ns/javaee references but we should list others as well in that issue so we can be sure to make a complete sweep) of them all. |
Also see |
As per https://eclipse-ee4j.github.io/jakartaee-platform/minutes/2020-01-28.html the requirement is: |
IMO, yes, as per https://eclipse-ee4j.github.io/jakartaee-platform/minutes/2020-01-28.html.
TCK challenges might later be filed by any implementations that doesn't think they are required to pass TCK tests that require schemas that are < Jakarta EE 8. IMO, the challenges would likely result in said TCK tests being excluding which lowers the bar for compatibility, which isn't good.
Updating the remaining TCK tests that still use older (pre Jakarta EE 8) to instead use Jakarta EE 9 schemas could introduce TCK test failures, so I think we need to get the TCK test schema updates in soon so that impacted SPEC API ballots can conclude. We also need to identify which TCKs are impacted (see comment from Lukas above that mentioned some of them). |
[javatest.batch] 08-14-2020 03:50:07: ERROR: Test case throws exception: com.sun.ts.lib.harness.EETest$Fault: Setup failed: The PU properties we pass is : [javatest.batch] 08-14-2020 03:50:07: TRACE: persistence unit properites from user: {jakarta.persistence.jdbc.password=cts1, jakarta.persistence.provider=org.eclipse.persistence.jpa.PersistenceProvider, eclipselink.logging.level=OFF, jakarta.persistence.jdbc.url=jdbc:derby://localhost:1527/derbyDB;create=true, jakarta.persistence.jdbc.driver=org.apache.derby.jdbc.ClientDriver, jakarta.persistence.jdbc.user=cts1} Can you suggest if we are missing any property or something else.
// <entity-mappings xmlns="https://jakarta.ee/xml/ns/persistence/orm" But it seems https://jakarta.ee/xml/ns/persistence/orm_3_0.xsd is not the correct link instead https://jakarta.ee/xml/ns/persistence/orm/orm_3_0.xsd is. Hence the below mapping was used : But both above mappings do give below exception for orm. Can you suggest how should the mapping be. [javatest.batch] Internal Exception: Exception [EclipseLink-7305] (Eclipse Persistence Services - 3.0.0.v202004301223): org.eclipse.persistence.exceptions.ValidationException |
@lukasj Is there a tracking issue/pullrequest for switching to https://jakarta.ee/xml/ns/persistence/orm/orm_3_0.xsd as mentioned in #2 in previous comment? |
nope, feel free to create it. note that orm_3.0.xsd is not the only one, at least persistence_3.0.xsd and probably jaxb cust schema may have to be adopted too |
@lukasj I created jakartaee/persistence#286 for a update in orm_3_0.xsd. Are there EclipseLink or other https://github.com/jakartaee/specifications issues needed? jakartaee/specifications#246 does seem to have https://github.com/jakartaee/specifications/pull/246/files#diff-579dce887502f6db9ebf8c8e758ccba4R16 which refers to orm_3_0.xsd. What additional changes are needed for persistence_3.0.xsd and probably jaxb cust schema? |
eclipselink as a compatible implementation included in GF does not support new schemas yet, that is the main thing at this point. |
I just added eclipse-ee4j/eclipselink#886 for updating EclipseLink to reference https://jakarta.ee/xml/ns/persistence/orm/orm_3_0.xsd in:
Sound right? |
@lukasj Persistence TCK is now promoted to https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/promoted/jakarta-persistence-tck-3.0.0.zip |
@lukasj do you want us to stage a new jakarta-persistence-tck-3.0.0.zip with the new changes from #537? If you then approve of the new TCK, we can promote that over https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/promoted/jakarta-persistence-tck-3.0.0.zip. |
@scottmarlow I think that would be the best thing to do but I'm not sure if we can do so, since the ballot is already running and I don't want it to be restarted for the 3rd time. Maybe @kwsutter knows if we can just go ahead and update TCK binary. Since tomitribe did not vote '-1' on the ballot due to this, it can probably wait for the next release (be it 3.0.1 or version for EE 10). Rerunning TCKs and updating checksums in existing PRs and CI requests is not a big deal on my end should the decision be to update the tck binary now. |
Thanks @lukasj , I agree that including the changes in the next TCK release will also be good. |
Complete userguide/documentation changes for Jakarta Persistence TCK.
Review the jakarta-persistence-tck-3.0.0.zip bundle at https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-900/ (or persistence-tck-3.0.0.zip bundle at https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9/nightly) to look for any changes missed to be included in JakartaEE 9 release.
Promote the bundle https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-900/jakarta-persistence-tck-3.0.0.zip to https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/promoted/ by using the job https://ci.eclipse.org/jakartaee-tck/job/promote_jakartaee9_eftl_bundles/.
Remove the boolean parameter (PERSISTENCE_TCK) and comment out the respective wget + if condition in https://ci.eclipse.org/jakartaee-tck/job/promote_jakartaee9_eftl_bundles/configure so that the same bundle is not promoted again.
The text was updated successfully, but these errors were encountered: