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

[improve][broker] Documentation for AuthenticationState contract #19283

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

michaeljmarshall
Copy link
Member

PIP 97: #12105

Motivation

While working on PIP 97, I noticed that we do not have a clear contract for the AuthenticationState interface. We use the interface in one way in the ServerCnx class, but some of our official implementations do not align with the usage and the API. The PR itself provides my proposed authoritative documentation.

When we agree on the content of this PR, I will follow up with some changes to the TokenAuthenticationState class and potentially others.

Modifications

  • Add documentation on how the AuthenticationState interface will be used by Pulsar.

Verifying this change

I have studied the code closely to understand how it works, and more importantly, how it should work.

Does this pull request potentially affect one of the following parts:

This change could affect how plugins interact with the AuthenticationState object.

Documentation

  • doc

This is a documentation change.

@michaeljmarshall michaeljmarshall added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. area/broker area/authn labels Jan 19, 2023
@michaeljmarshall michaeljmarshall added this to the 2.12.0 milestone Jan 19, 2023
@michaeljmarshall michaeljmarshall self-assigned this Jan 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2023

Codecov Report

Merging #19283 (1709da8) into master (334c3a5) will increase coverage by 5.83%.
The diff coverage is 25.31%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19283      +/-   ##
============================================
+ Coverage     47.04%   52.87%   +5.83%     
- Complexity     9190    22399   +13209     
============================================
  Files           607     1824    +1217     
  Lines         57677   136687   +79010     
  Branches       6007    15044    +9037     
============================================
+ Hits          27132    72275   +45143     
- Misses        27598    56913   +29315     
- Partials       2947     7499    +4552     
Flag Coverage Δ
inttests 22.47% <6.63%> (?)
systests 24.82% <7.46%> (?)
unittests 47.08% <24.48%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...sar/broker/authentication/AuthenticationState.java 9.09% <ø> (ø)
...rg/apache/pulsar/broker/delayed/bucket/Bucket.java 0.00% <0.00%> (ø)
...r/delayed/bucket/BucketDelayedDeliveryTracker.java 0.00% <0.00%> (ø)
...layed/bucket/CombinedSegmentDelayedIndexQueue.java 0.00% <0.00%> (ø)
...ulsar/broker/delayed/bucket/DelayedIndexQueue.java 0.00% <0.00%> (ø)
.../pulsar/broker/delayed/bucket/ImmutableBucket.java 0.00% <0.00%> (ø)
...he/pulsar/broker/delayed/bucket/MutableBucket.java 0.00% <0.00%> (ø)
...ed/bucket/TripleLongPriorityDelayedIndexQueue.java 0.00% <0.00%> (ø)
...ar/broker/service/persistent/ShadowReplicator.java 12.00% <0.00%> (-0.25%) ⬇️
...rg/apache/pulsar/client/impl/PulsarClientImpl.java 55.55% <0.00%> (+10.75%) ⬆️
... and 1374 more

@nodece nodece merged commit de3b855 into apache:master Jan 20, 2023
@michaeljmarshall michaeljmarshall deleted the pip-97-document-auth-state branch January 20, 2023 19:58
michaeljmarshall added a commit to michaeljmarshall/pulsar that referenced this pull request Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/authn area/broker doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants