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

[JBTM-3216] narrow and fix recovery setup in jbossts-properties.xml descriptors #1524

Conversation

ochaloup
Copy link
Contributor

https://issues.jboss.org/browse/JBTM-3216
forum at: https://developer.jboss.org/message/991332

MAIN QA_JTA QA_JTS_OPENJDKORB
!QA_JTS_JDKORB !QA_JTS_JACORB !BLACKTIE !XTS !PERF NO_WIN !RTS !AS_TESTS !TOMCAT !JACOCO !LRA

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@mmusgrov
Copy link
Member

Do you have any evidence to justify this change of behaviour. If none of our users and stakeholders have reported an issue I think we need to be cautious about removing filters.

@ochaloup
Copy link
Contributor Author

@mmusgrov yes. The filters are put wrongly as nobody was caring how they are defined in the xml descriptors.
If you have some worries, I'm really interested in. Just, please, comment on the particular change/filter. Thanks.

@mmusgrov
Copy link
Member

@mmusgrov yes. The filters are put wrongly as nobody was caring how they are defined in the xml descriptors.
If you have some worries, I'm really interested in. Just, please, comment on the particular change/filter. Thanks.

I was referring to the removal of filters:

  1. Will removing a filter change the behaviour for the end use?
  2. How do you know that it is safe to remove a filter?
  3. Why do you need to remove a filter - is there a question/issue raised by a community member due to the presence of the filter?
  4. Is the functionality provided by the removed filter available elsewhere or is it simply not required anymore?

@ochaloup
Copy link
Contributor Author

@mmusgrov yes. The filters are put wrongly as nobody was caring how they are defined in the xml descriptors.
If you have some worries, I'm really interested in. Just, please, comment on the particular change/filter. Thanks.

I was referring to the removal of filters:

1. Will removing a filter change the behaviour for the end use?

No. The filters which are to be removed are either not used as the JTA standalone application is not configured to run JCA or ejb remote calls or are not used at all as JTS does not work with JTA.

2. How do you know that it is safe to remove a filter?

As the the functionality of the filters can be studied in the code and they are either not used (as for JTS they work with JTA transactions) or they does not make sense (as for JTA standalone app is not used Java EE functionality)

3. Why do you need to remove a filter - is there a question/issue raised by a community member due to the presence of the filter?

I want to remove the filters to make configuration clear for the user. This change defines what is the recommended from Narayana to use.
I raised the question at the forum as I consider it wrong and unmaintained.

4. Is the functionality provided by the removed filter available elsewhere or is it simply not required anymore?

The functionality is needed for use cases which are not relevant for the particular configuration. Please, take a look into code and verify.

@mmusgrov mmusgrov self-requested a review October 31, 2019 09:28
Copy link
Member

@mmusgrov mmusgrov left a comment

Choose a reason for hiding this comment

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

Thanks for the answers to my questions (I did peruse the forum thread but was unable to find the answers that I was looking for, hence the query on the PR).

@ochaloup
Copy link
Contributor Author

Thanks @mmusgrov.

I consider this could be merged as reviewed by Mike and the changes reflects our discussion with @tomjenkinson at the forum.

@ochaloup ochaloup merged commit 21e61ff into jbosstm:master Oct 31, 2019
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.

3 participants