Agroal - Allow to configure transaction isolation level #1122

[machi1990]

The PR adds the possibility to define database transaction isolation level using quarkus.datasource.transaction-isolation config property.

@gsmet

[gsmet]

@machi1990 nice work. I just added a comment regarding the config property name. I think we'd better be explicit.

[gsmet]

I think I would have named it transaction-isolation-level.

[machi1990]

Done.

[gsmet]

@machi1990 I'm wondering if we should have our own enum for the isolation level values. Right now, we are totally tied to the Agroal values and I think it would be better if we had our own enum.

I think I would have string values like read-committed, read-uncommitted and the TransactionIsolationLevel enum would have a public static TransactionIsolationLevel of(String value) method so that it automatically is transformed by MicroProfile Config.

We would then translate the values to the Agroal values.

What do you think of this proposal?

[machi1990]

@gsmet, I like the idea. It will totally abstract the end user from the Agroal’s transaction isolation level enum. I’ll take this into account.

A small downside will be maintaining our own enum, should there be any breaking change in Agroal.

[gsmet]

I'm waiting for #1815 to get in so that the enum is not formatted as it is right now as it's barely readable.

[machi1990]

I'm waiting for #1815 to get in so that the enum is not formatted as it is right now as it's barely readable.

Okay. I'll rebase afterwards.

[gsmet]

@machi1990 don't worry about it, I already have everything ready.

[gsmet]

@machi1990 I just pushed a rebase to your branch with the new formatting. I'm waiting for CI and will merge.

Thanks a lot for this one.

[machi1990]

@machi1990 I just pushed a rebase to your branch with the new formatting. I'm waiting for CI and will merge.

Thanks a lot for this one.

Cool. Thanks for the valuable feedback.

[Sanne]

thanks @machi1990 and @gsmet ! Looks very useful

[gsmet]

Merged, thanks @machi1990 !