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

NTA-89: Add a keyword filter #80

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

NTA-89: Add a keyword filter #80

wants to merge 2 commits into from

Conversation

Yuan-Hu
Copy link
Contributor

@Yuan-Hu Yuan-Hu commented Apr 29, 2016

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/75/

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/76/

inputStream = getClass().getClassLoader().getResourceAsStream(propFileName);

if (inputStream != null) {
prop.load(inputStream);
Copy link
Member

Choose a reason for hiding this comment

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

Every time getPropValues() get's called you are re-reading the properties. How about just doing it once in a constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/77/

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/78/

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/79/

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/80/

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/81/

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/82/

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/83/

filterKeywords = new ArrayList<>();

String propFileName = "filter.properties";
logger.warn(getClass().getClassLoader().getResource(""));
Copy link
Member

Choose a reason for hiding this comment

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

This log message does not server any purpose.

@jbosstm-bot
Copy link

Started testing this pull request: http://albany.eng.hst.ams2.redhat.com/job/pulls-transaction-analyser/84/

@mmusgrov
Copy link
Member

mmusgrov commented May 6, 2016

@Yuan-Hu The changes are looking much better now, thank you for you patience.

@zhfeng The associated jira (NTA-89) does not give enough information to determine whether the implementation does what is required. Is the intention to only consider for parsing those lines that match the comma separated keywords list? Or is this some kind of processing pipeline where other filtering has already happened?

@Yuan-Hu
Copy link
Contributor Author

Yuan-Hu commented May 6, 2016

@mmusgrov Also thanks for your continuous advice. And NTA-89 is created by myself for the former intention you mentioned.

@@ -0,0 +1,2 @@
keywords=TransactionImple,Periodic Recovery,TransactionSynchronizationRegistryImple

Copy link
Member

Choose a reason for hiding this comment

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

How did you decide on this list of keywords. If they are controlling which log lines are passed on for further processing then there are many others that need to be included, for example BasicAction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, they are controlling the lines we don't need.

Copy link
Member

Choose a reason for hiding this comment

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

That's exactly the reason why the JIRA is incomplete. A good JIRA will explain the requirement and perhaps where the requirement originates from.

Why is there not a need for filters that block lines and other filters that match lines. If there is such a need then you would need at least two property names, for example: matching_keywords and blocked_keywords

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/1/

@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/4/

@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/5/

@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/6/

@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/2/

1 similar comment
@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/2/

@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/3/

@jbosstm-bot
Copy link

Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/1/

@mmusgrov
Copy link
Member

mmusgrov commented Jan 3, 2023

@Yuan-Hu I am closing this PR since the Blacktie project is now archived (https://groups.google.com/g/narayana-users/c/k0EgFlS6VJc/m/6DcliSBfEAAJ). But thanks very much for your PR. What are you using for a transaction manager now?

@mmusgrov mmusgrov closed this Jan 3, 2023
@mmusgrov
Copy link
Member

mmusgrov commented Jan 3, 2023

Sorry @Yuan-Hu I got the analyser confused with the archived BlackTie project - I have re-opened your PR.

@mmusgrov mmusgrov reopened this Jan 3, 2023
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