Skip to content

add @Recoverable#238

Open
gavinking wants to merge 1 commit intojakartaee:masterfrom
gavinking:#227
Open

add @Recoverable#238
gavinking wants to merge 1 commit intojakartaee:masterfrom
gavinking:#227

Conversation

@gavinking
Copy link
Copy Markdown
Member

for #227

@jta-bot
Copy link
Copy Markdown
Contributor

jta-bot commented Sep 22, 2025

Can one of the admins verify this patch?

@gavinking
Copy link
Copy Markdown
Member Author

Note that I will have to redo this PR after #239 is merged (if it is merged).

@tomjenkinson
Copy link
Copy Markdown
Contributor

test this please

@tomjenkinson tomjenkinson self-requested a review September 26, 2025 09:05
@tomjenkinson
Copy link
Copy Markdown
Contributor

We do need a TCK test (similar to https://github.com/jakartaee/platform-tck/pull/2163/files) and need to wait a bit more for the discussion to elapse the 7 days I offered (https://www.eclipse.org/lists/jta-dev/msg00373.html), but I will approve this from my point of view.

@tomjenkinson
Copy link
Copy Markdown
Contributor

In terms of TCK tests we think we need tests for the following scenarios:

  • One that shows a recoverable exception does not rollback the transaction
  • One that shows a recoverable exception can be made to rollback the transaction (by overriding the default)

I can help with the integration, but please can I ask for if you can provide something I can try to work which shows these general scenarios.

@scottmarlow
Copy link
Copy Markdown
Member

We do need a TCK test (similar to https://github.com/jakartaee/platform-tck/pull/2163/files) and need to wait a bit more for the discussion to elapse the 7 days I offered (https://www.eclipse.org/lists/jta-dev/msg00373.html), but I will approve this from my point of view.

Before we can merge Platform TCK test changes that depend on Jakarta Transactions 2.1 we will need a SPEC API snapshot that covers what the test change will need.

In case it helps jakartaee/servlet#924 is for Update parent POM to 2.0.0-SNAPSHOT and switch to Central for SNAPSHOTs which may be helpful in preparing to push Transactions 2.1 SPEC API SNAPSHOTs.

@gavinking
Copy link
Copy Markdown
Member Author

Before we can merge Platform TCK test changes that depend on Jakarta Transactions 2.1 we will need a SPEC API snapshot that covers what the test change will need.

Good point, there's a chicken/egg problem here, I guess.

@scottmarlow
Copy link
Copy Markdown
Member

Before we can merge Platform TCK test changes that depend on Jakarta Transactions 2.1 we will need a SPEC API snapshot that covers what the test change will need.

Good point, there's a chicken/egg problem here, I guess.

Very true but from a development point of view, we can push a snapshot of the Transactions 2.1 SPEC API multiple times so that the EE 12 BOM snapshot can be updated and the Platform TCK can than use the EE 12 BOM that includes each Transactions spec snapshot update.

I think that the Transaction 2.1 specification will be ratified when we have an Transaction 2.1 implementation that can be tested in an EE implementation (implementing Transaction 2.1) that passes a snapshot build of the Transaction 2.1 (or EE 12) TCK. Typically we pass snapshot/staged TCKs before the ratification (with the TCK hash sum identified) and ensure that same TCK is the one that is promoted before the ratification completes. Or something like that.

@jhanders34 @jamezp does ^ make sense?

@jamezp
Copy link
Copy Markdown
Member

jamezp commented Oct 16, 2025

Sorry for the delay on this @scottmarlow, it got lost in me email. I'm not sure what the best option is here. A SNAPSHOT seems fine. I will say though this is a perfect example of why having the TCK tests within the spec repository makes sense :)

@scottmarlow
Copy link
Copy Markdown
Member

A SNAPSHOT seems fine. I will say though this is a perfect example of why having the TCK tests within the spec repository makes sense :)

+1 although we could also start with a 2.1 milestone release as well.

#242 is for updating the Transactions repo parent pom to 2.0.0-M1 and also adds org.sonatype.central:central-publishing-maven-plugin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants