Skip to content

Conversation

garyrussell
Copy link
Contributor

Resolves #101

@artembilan
Copy link
Member

I think I'm fine with the solution.

@mbogoevici ?


private final AcknowledgingMessageListener<K, V> delegate;

private final boolean ackDiscarded;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we add this option into the MessagingMessageListenerAdapter as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - I was hoping to avoid that 😄

Choose a reason for hiding this comment

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

Trying to imagine a use case where not acknowledging a discarded message would be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They might be acking manually on some condition (special marker record perhaps and not want random acks being done on their behalf.

I guess I am just a bit uncomfortable issuing automatic acks when they have specifically requested to use manual acks.

Copy link

@mbogoevici mbogoevici Jun 1, 2016

Choose a reason for hiding this comment

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

Ah, I see. An option gives a choice then. Should we at least make this true by default - otherwise, in the case of manual acks, they would almost never have a chance to ack those messages and there's no chance that they would ack them in the future on replay - so unless there's a message that they're interested in, this would keep redelivering messages on each restart.

@garyrussell
Copy link
Contributor Author

Pushed - @artembilan - it's a bit simpler now, if the container listener factory is configured - we wrap the MMLA with a filtering adapter.

@garyrussell
Copy link
Contributor Author

The trouble with making it true by default is it really can only be used of the AckMode is set to one of the MANUAL options.

}
}
else {
container.setupMessageListener(messageListener);
Copy link
Member

Choose a reason for hiding this comment

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

We can still have this line of code without duplication in each if...else.
Minor, of course.
Will polish on merge.

@artembilan
Copy link
Member

Merged as b1dc76e

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