Skip to content

Conversation

@michaeljmarshall
Copy link
Member

Motivation

In #19975, we introduced a wrapper for all authentication parameters. This PR adds that wrapper to the Rest Producer.

Modifications

  • Use AuthenticationParameters to simplify parameter management in Rest Producer.
  • Add method to the AuthorizationService that takes the AuthenticationParameters.
  • Update annotations on Rest Producer to indicate that a 401 is an expected response.

Verifying this change

This change is covered by the TopicsAuthTest.

Documentation

  • doc-not-needed

This is an internal change that does not need to be documented.

Matching PR in forked repository

PR in forked repository: skipping PR since the relevant tests pass locally

@michaeljmarshall michaeljmarshall added area/admin doc-not-needed Your PR changes do not impact docs type/refactor Code or documentation refactors. e.g. refactor code structure or methods to improve code readability ready-to-test labels Apr 8, 2023
@michaeljmarshall michaeljmarshall self-assigned this Apr 8, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #20046 (bc8fae1) into master (3b118b6) will increase coverage by 0.00%.
The diff coverage is 44.80%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #20046   +/-   ##
=========================================
  Coverage     72.86%   72.86%           
- Complexity    31628    31643   +15     
=========================================
  Files          1861     1858    -3     
  Lines        137500   137434   -66     
  Branches      15141    15106   -35     
=========================================
- Hits         100187   100143   -44     
+ Misses        29351    29312   -39     
- Partials       7962     7979   +17     
Flag Coverage Δ
inttests 24.38% <0.92%> (-0.01%) ⬇️
systests 25.03% <19.39%> (+0.02%) ⬆️
unittests 72.15% <36.02%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
...a/org/apache/pulsar/broker/admin/v2/Functions.java 0.00% <0.00%> (ø)
...java/org/apache/pulsar/broker/admin/v2/Worker.java 11.76% <0.00%> (ø)
...org/apache/pulsar/broker/admin/v2/WorkerStats.java 40.00% <0.00%> (ø)
...ain/java/org/apache/pulsar/broker/rest/Topics.java 100.00% <ø> (ø)
...ons/worker/rest/api/v2/FunctionsApiV2Resource.java 0.00% <0.00%> (ø)
...s/worker/rest/api/v2/WorkerStatsApiV2Resource.java 0.00% <0.00%> (ø)
...ulsar/functions/worker/rest/api/FunctionsImpl.java 56.55% <16.66%> (-0.74%) ⬇️
...org/apache/pulsar/broker/admin/impl/SinksBase.java 35.29% <35.71%> (ø)
...g/apache/pulsar/broker/admin/impl/SourcesBase.java 35.29% <35.71%> (-1.85%) ⬇️
...ons/worker/rest/api/v3/FunctionsApiV3Resource.java 43.39% <37.50%> (ø)
... and 17 more

... and 68 files with indirect coverage changes

@michaeljmarshall michaeljmarshall merged commit 7990948 into apache:master Apr 10, 2023
@michaeljmarshall michaeljmarshall deleted the refactor-rest-api-producer branch April 10, 2023 17:13
@michaeljmarshall michaeljmarshall added this to the 3.0.0 milestone Apr 10, 2023
michaeljmarshall added a commit that referenced this pull request Apr 10, 2023
…0046)

In #19975, we introduced a wrapper for all authentication parameters. This PR adds that wrapper to the Rest Producer.

* Use `AuthenticationParameters` to simplify parameter management in Rest Producer.
* Add method to the `AuthorizationService` that takes the `AuthenticationParameters`.
* Update annotations on Rest Producer to indicate that a 401 is an expected response.

This change is covered by the `TopicsAuthTest`.

- [x] `doc-not-needed`

This is an internal change that does not need to be documented.

PR in forked repository: skipping PR since the relevant tests pass locally

(cherry picked from commit 7990948)
michaeljmarshall added a commit that referenced this pull request Apr 10, 2023
…0046)

In #19975, we introduced a wrapper for all authentication parameters. This PR adds that wrapper to the Rest Producer.

* Use `AuthenticationParameters` to simplify parameter management in Rest Producer.
* Add method to the `AuthorizationService` that takes the `AuthenticationParameters`.
* Update annotations on Rest Producer to indicate that a 401 is an expected response.

This change is covered by the `TopicsAuthTest`.

- [x] `doc-not-needed`

This is an internal change that does not need to be documented.

PR in forked repository: skipping PR since the relevant tests pass locally

(cherry picked from commit 7990948)
(cherry picked from commit 02b27e8)
michaeljmarshall added a commit that referenced this pull request Apr 10, 2023
…0046)

In #19975, we introduced a wrapper for all authentication parameters. This PR adds that wrapper to the Rest Producer.

* Use `AuthenticationParameters` to simplify parameter management in Rest Producer.
* Add method to the `AuthorizationService` that takes the `AuthenticationParameters`.
* Update annotations on Rest Producer to indicate that a 401 is an expected response.

This change is covered by the `TopicsAuthTest`.

- [x] `doc-not-needed`

This is an internal change that does not need to be documented.

PR in forked repository: skipping PR since the relevant tests pass locally

(cherry picked from commit 7990948)
(cherry picked from commit 02b27e8)
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Apr 11, 2023
…ache#20046)

### Motivation

In apache#19975, we introduced a wrapper for all authentication parameters. This PR adds that wrapper to the Rest Producer.

### Modifications

* Use `AuthenticationParameters` to simplify parameter management in Rest Producer.
* Add method to the `AuthorizationService` that takes the `AuthenticationParameters`.
* Update annotations on Rest Producer to indicate that a 401 is an expected response.

### Verifying this change

This change is covered by the `TopicsAuthTest`.

### Documentation

- [x] `doc-not-needed` 

This is an internal change that does not need to be documented.

### Matching PR in forked repository

PR in forked repository: skipping PR since the relevant tests pass locally
michaeljmarshall added a commit to datastax/pulsar that referenced this pull request Apr 11, 2023
…ache#20046)

In apache#19975, we introduced a wrapper for all authentication parameters. This PR adds that wrapper to the Rest Producer.

* Use `AuthenticationParameters` to simplify parameter management in Rest Producer.
* Add method to the `AuthorizationService` that takes the `AuthenticationParameters`.
* Update annotations on Rest Producer to indicate that a 401 is an expected response.

This change is covered by the `TopicsAuthTest`.

- [x] `doc-not-needed`

This is an internal change that does not need to be documented.

PR in forked repository: skipping PR since the relevant tests pass locally

(cherry picked from commit 7990948)
(cherry picked from commit 02b27e8)
@michaeljmarshall
Copy link
Member Author

As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/admin cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 cherry-picked/branch-2.11 doc-not-needed Your PR changes do not impact docs ready-to-test release/2.10.4 release/2.11.1 type/refactor Code or documentation refactors. e.g. refactor code structure or methods to improve code readability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants