Skip to content
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

Adding tests to create and verify an object-store directory #15629

Merged
merged 1 commit into from
Mar 19, 2021
Merged

Adding tests to create and verify an object-store directory #15629

merged 1 commit into from
Mar 19, 2021

Conversation

mayankkunwar
Copy link
Contributor

@quarkus-bot quarkus-bot bot added the area/narayana Transactions / Narayana label Mar 11, 2021
Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this property is only effective for XA transactions, this should be noted in the documentation.
What about using "quarkus.datasource.jdbc.transactions=XA"?

@mayankkunwar
Copy link
Contributor Author

We create object-store when needed, not for every transaction, object-store would be created. I am not sure what to document, whenever there is a need of object-store internal to JTA-transaction we create it, otherwise we don't.

The issue was to just set the path of object-store whenever it is created.

Base automatically changed from master to main March 12, 2021 15:55
@rsvoboda
Copy link
Member

@mayankkunwar I think Jose's concern is that this test is not using Quarkus application with the "real" scenario but just focuses on the minimal code needed to trigger creation of the object-store directory.

Agree that more "real" scenario would be good addition to the test coverage so that Quarkus users can make easy use of it. For some developers it can a bit frightening when the need for transactions appears in their project and it is always nice to have good test/example to look into.

Should be that more "real" scenario addressed in this PR or it is fine to create followup GH issue to address it later?
@Sgitario / @mayankkunwar wdyt?

@mayankkunwar
Copy link
Contributor Author

@rsvoboda Thanks for the clarification. I think there is already a issue created #9623 . Maybe we could add these tests as part of the mentioned issue?

@rsvoboda
Copy link
Member

+1, I'm going to add approval for this PR

@gsmet gsmet merged commit 8f66aca into quarkusio:main Mar 19, 2021
@quarkus-bot quarkus-bot bot added this to the 1.14 - main milestone Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/narayana Transactions / Narayana
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants