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

Assignment test + NPE fix #490

Merged
merged 5 commits into from
Jun 1, 2021
Merged

Conversation

sknot-rh
Copy link
Member

@sknot-rh sknot-rh commented Nov 9, 2020

Signed-off-by: Stanislav Knot [email protected]

Assigment endpoint does not have any tests.
Trying to access subscribe and assign endpoints ends up with NPE. After fix of NPE, it does not return 409 (Conflict).

Fixes #489

@sknot-rh sknot-rh requested a review from ppatierno November 9, 2020 11:52
@sknot-rh sknot-rh added this to the 0.20.0 milestone Nov 9, 2020
@sknot-rh sknot-rh requested a review from im-konge November 9, 2020 14:14
Copy link
Member

@im-konge im-konge left a comment

Choose a reason for hiding this comment

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

LGTM, maybe there is something that @ppatierno will see :)

@sknot-rh sknot-rh requested a review from a team November 9, 2020 15:00
@@ -357,6 +357,7 @@ private void partitionsAssignmentAndSeek() {
Optional<PartitionInfo> requestedPartitionInfo =
availablePartitions.stream()
.filter(p -> p.getTopic().equals(topicSubscription.getTopic()) &&
topicSubscription.getPartition() != null &&
Copy link
Member

Choose a reason for hiding this comment

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

can you explain in which case this can be null?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we do subscribe to a topic, it is stored into topicSubscriptions. Subscription request body might be for example

{
  "topics": [
    "topic1",
    "topic2"
  ]
}

If we do assign after subscription (what is wrong because of exclusivity), we iterate through all topic subscriptions in topicSubscriptions and we are trying to get partitions. And because the subscription body does not have any partitions info, it is null. Then just comparing null causes NPE.

@sknot-rh sknot-rh requested a review from ppatierno November 24, 2020 12:58
Base automatically changed from master to main March 25, 2021 19:24
sknot-rh added 5 commits May 24, 2021 15:03
Signed-off-by: Stanislav Knot <[email protected]>
Signed-off-by: Stanislav Knot <[email protected]>
Signed-off-by: Stanislav Knot <[email protected]>
Signed-off-by: Stanislav Knot <[email protected]>
@sknot-rh sknot-rh force-pushed the assignment-fixes branch from fb56fc8 to d0219bc Compare May 24, 2021 13:18
@scholzj scholzj merged commit f96cb3f into strimzi:main Jun 1, 2021
@sknot-rh sknot-rh deleted the assignment-fixes branch June 16, 2021 12:41
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.

Assignment is broken
4 participants