Skip to content

Comments

CAMEL-13601 - Fix camel-jira client close and add logging#2952

Closed
claudio4j wants to merge 1 commit intoapache:masterfrom
claudio4j:jira_fixes
Closed

CAMEL-13601 - Fix camel-jira client close and add logging#2952
claudio4j wants to merge 1 commit intoapache:masterfrom
claudio4j:jira_fixes

Conversation

@claudio4j
Copy link
Contributor

super(endpoint, processor);
LOG.info("JIRA NewCommentsConsumer: Indexing current issue comments...");
getComments(false);
LOG.info("JIRA NewCommentsConsumer: Indexing current issue comments.");
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move such tasks to doStart and not in constructors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks for the review.

// Ignore maxResults if it's <= 0.
protected List<Issue> getIssues(String jql, int start, int maxPerQuery, int maxResults) {
LOG.info("Indexing current JIRA issues...");
LOG.info("Start indexing current JIRA issues...");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need this INFO logging noise? If this method is invoked frequently you get spammed with logs. Can we lower this to DEBUG

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks for the review.

Stack<Comment> newComments = new Stack<>();
@SuppressWarnings("ConstantConditions")
private List<Comment> getComments() {
LOG.info("Start: Jira NewCommentsConsumer: retrieving issue comments. Last comment id: {}", lastCommentId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need this INFO logging noise? If this method is invoked frequently you get spammed with logs. Can we lower this to DEBUG

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

I don't know why changing the guava version

<jgroups-raft-leveldbjni-version>1.8</jgroups-raft-leveldbjni-version>
<jgroups-raft-mapdb-version>1.0.8</jgroups-raft-mapdb-version>
<jira-guava-version>26.0-jre</jira-guava-version>
<jira-guava-version>20.0</jira-guava-version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this one was changed ? Just curiosity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The jira rest java client (JRJC) 5.1.0 doesn't define a guava version. As previously I used JRJC 5.1.2, its pom.xml defined the guava version, but when I changed to 5.1.0, I forgot to add the guava version.
https://bitbucket.org/atlassian/jira-rest-java-client/src/jira-rest-java-client-parent-5.1.0/pom.xml

Copy link
Contributor

Choose a reason for hiding this comment

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

ok. thanks

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

Probably all the logging stuff can be at DEBUG level

@claudio4j
Copy link
Contributor Author

Done, thanks for the review.

@oscerd
Copy link
Contributor

oscerd commented May 31, 2019

What's the reason for Guava version downgrade?

@oscerd
Copy link
Contributor

oscerd commented Jun 3, 2019

Merged on master

@oscerd
Copy link
Contributor

oscerd commented Jun 3, 2019

Thanks

@oscerd oscerd closed this Jun 3, 2019
@claudio4j claudio4j deleted the jira_fixes branch June 3, 2019 12:26
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