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

KAFKA-9392; Clarify deleteAcls javadoc and add test for create/delete timing #7956

Merged
merged 2 commits into from
Jun 1, 2020

Conversation

rajinisivaram
Copy link
Contributor

Follow-on from PR #7911 to clarify the guarantee for deleteAcls and add a deterministic test to ensure we don't regress.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@rajinisivaram
Copy link
Contributor Author

retest this please

* returned {@link AclDeleteResult} indicates which ACLs were deleted.</li>
* <li>If the provided filters use other resource pattern types that perform a direct match, all matching ACLs
* from previously completed {@link #createAcls(AuthorizableRequestContext, List)} )} are guaranteed
* to be deleted.</li>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ijuma I added these to the Authorizer API (hence Admin API as well). But I wasn't sure if we should leave it to the implementation instead - i.e. implementations may provider stronger or weaker guarantees. If so, this doc would move to AclAuthorizer. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, maybe we can move this doc to AclAuthorizer. we can also mention here to refer implementation docs for concurrent update guarantees.

@rajinisivaram
Copy link
Contributor Author

retest this please

@ijuma
Copy link
Contributor

ijuma commented May 28, 2020

@omkreddy Maybe you can help review this.

@ijuma
Copy link
Contributor

ijuma commented May 28, 2020

@rajinisivaram there are some conflicts.

@rajinisivaram
Copy link
Contributor Author

rajinisivaram commented May 28, 2020

Thanks @ijuma , rebased on trunk.

Copy link
Contributor

@omkreddy omkreddy left a comment

Choose a reason for hiding this comment

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

@rajinisivaram Thanks for the PR. LGTM.

@rajinisivaram
Copy link
Contributor Author

@omkreddy Thanks for the review. Moved the comment to AclAuthorizer. Merging to trunk.

@rajinisivaram rajinisivaram merged commit 66fdb59 into apache:trunk Jun 1, 2020
rajinisivaram added a commit that referenced this pull request Jun 1, 2020
… timing (#7956)

Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
ijuma added a commit to confluentinc/kafka that referenced this pull request Jun 3, 2020
* apache-github/2.6: (32 commits)
  KAFKA-10083: fix failed testReassignmentWithRandomSubscriptionsAndChanges tests (apache#8786)
  KAFKA-9945: TopicCommand should support --if-exists and --if-not-exists when --bootstrap-server is used (apache#8737)
  KAFKA-9320: Enable TLSv1.3 by default (KIP-573) (apache#8695)
  KAFKA-10082: Fix the failed testMultiConsumerStickyAssignment (apache#8777)
  MINOR: Remove unused variable to fix spotBugs failure (apache#8779)
  MINOR: ChangelogReader should poll for duration 0 for standby restore (apache#8773)
  KAFKA-10030: Allow fetching a key from a single partition (apache#8706)
  Kafka-10064 Add documentation for KIP-571 (apache#8760)
  MINOR: Code cleanup and assertion message fixes in Connect integration tests (apache#8750)
  KAFKA-9987: optimize sticky assignment algorithm for same-subscription case (apache#8668)
  KAFKA-9392; Clarify deleteAcls javadoc and add test for create/delete timing (apache#7956)
  KAFKA-10074: Improve performance of `matchingAcls` (apache#8769)
  KAFKA-9494; Include additional metadata information in DescribeConfig response (KIP-569) (apache#8723)
  KAFKA-10056; Ensure consumer metadata contains new topics on subscription change (apache#8739)
  KAFKA-10029; Don't update completedReceives when channels are closed to avoid ConcurrentModificationException (apache#8705)
  KAFKA-10061; Fix flaky `ReassignPartitionsIntegrationTest.testCancellation` (apache#8749)
  KAFKA-9130; KIP-518 Allow listing consumer groups per state (apache#8238)
  KAFKA-9501: convert between active and standby without closing stores (apache#8248)
  MINOR: Relax Percentiles test (apache#8748)
  MINOR: regression test for task assignor config (apache#8743)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants