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

Fix updating a non unique consumer on workqueue stream not returning an error #4654

Merged
merged 1 commit into from Oct 12, 2023

Conversation

mdawar
Copy link
Contributor

@mdawar mdawar commented Oct 12, 2023

This is a possible fix for #4653.

Changes made:

  1. Added tests for creating and updating consumers on a work queue stream with overlapping subjects.
  2. Check for overlapping subjects before updating the consumer config.
  3. Changed func (*stream).partitionUnique(partitions []string) bool to accept the consumer name being checked so we can skip it while checking for overlapping subjects (Required for FilterSubjects updates), wasn't needed before because the checks were made on creation only.

There's only 1 thing that I'm not sure about.

In the current work queue stream conflict checks, the consumer config Direct is being checked if false, should we also make this check before the update?

Signed-off-by: Pierre Mdawar pierre@mdawar.dev

@mdawar mdawar requested a review from a team as a code owner October 12, 2023 09:41
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 for the contribution as always.

@derekcollison derekcollison merged commit 1e8f6bf into nats-io:main Oct 12, 2023
2 checks passed
@mdawar mdawar deleted the overlapping-consumer-subjects branch October 12, 2023 15:46
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

2 participants