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

Add account, stream and consumer name to consumer alignment cleanup warning #3666

Merged
merged 1 commit into from Nov 25, 2022

Conversation

ch629
Copy link
Contributor

@ch629 ch629 commented Nov 25, 2022

  • Link to issue, e.g. Resolves #NNN
  • Documentation added (if applicable)
  • Tests added
  • Branch rebased on top of current main (git pull --rebase origin main)
  • Changes squashed to a single commit (described here)
  • Build is green in Travis CI
  • You have certified that the contribution is your original work and that you license the work to the project under the Apache 2 license

Resolves #

Changes proposed in this pull request:

  • Adds account, stream and consumer names to the consumer assignment not cleaned up warning

/cc @nats-io/core

@wallyqs wallyqs changed the base branch from main to dev November 25, 2022 19:06
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

Looks good, just minor nit on punctuation.

@@ -1362,7 +1362,7 @@ func (o *consumer) deleteNotActive() {
js.mu.RUnlock()
if ca != nil {
if fs {
s.Warnf("Consumer assignment not cleaned up, retrying")
s.Warnf("Consumer assignment not cleaned up, for account: '%s', stream: '%s', consumer: '%s', retrying", acc, stream, name)
Copy link
Member

Choose a reason for hiding this comment

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

Would remove ',' after "cleaned up", otherwise looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I didn't reread it fully after changing it, done

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks!

@derekcollison derekcollison merged commit be0558c into nats-io:dev Nov 25, 2022
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.

None yet

3 participants