Skip to content

fix(pulsar): Use topic_refresh_secs always#56

Open
darinspivey wants to merge 2 commits intomasterfrom
darinspivey/LOG-23134
Open

fix(pulsar): Use topic_refresh_secs always#56
darinspivey wants to merge 2 commits intomasterfrom
darinspivey/LOG-23134

Conversation

@darinspivey
Copy link
Contributor

chore(pulsar): Update comments regarding acks around failures

The code in place for acking upon failures is correct. There's some
confusion with how Kafka works versus Pulsar, so this adds appropriate
comments to indicate that although they operate acks differently, the
end result is essentially the same when event delivery fails.

Ref: LOG-23134


fix(pulsar): Use topic_refresh_secs always

Refreshing the topics list periodically now happens both with regex
subscriptions and without. This refresh value is now used across the
board, so place it in the main part of the builder.

Ref: LOG-23134

Refreshing the topics list periodically now happens both with regex
subscriptions and without. This refresh value is now used across the
board, so place it in the main part of the builder.

Ref: LOG-23134
The code in place for acking upon failures is correct. There's some
confusion with how Kafka works versus Pulsar, so this adds appropriate
comments to indicate that although they operate acks differently, the
end result is essentially the same when event delivery fails.

Ref: LOG-23134
Copy link
Member

@mdeltito mdeltito left a comment

Choose a reason for hiding this comment

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

change and comments LGTM

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.

2 participants