Skip to content

Conversation

@grssam
Copy link
Contributor

@grssam grssam commented Sep 4, 2024

This PR is a followup to #23120 to make the backward incompatible change to:

  • Make the default value of unload-scope to changed
  • Change behavior of all_matching to cover both removed and added regexes

PIP: #23116

This change is only meant for pulsar 4.x release

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added test case to validate behavior of all_matching flag
  • Added test case to verify default changed to changed

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: grssam#3

…s-isolation-policy set`

Also change behavior of all_matching to cover both removed and added regexes
@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Sep 4, 2024
@lhotari lhotari added this to the 4.0.0 milestone Sep 5, 2024
@lhotari lhotari self-requested a review September 5, 2024 11:37
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@grssam
Copy link
Contributor Author

grssam commented Sep 5, 2024

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.54%. Comparing base (bbc6224) to head (799ed24).
⚠️ Report is 1483 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23253      +/-   ##
============================================
+ Coverage     73.57%   74.54%   +0.96%     
- Complexity    32624    34289    +1665     
============================================
  Files          1877     1926      +49     
  Lines        139502   145039    +5537     
  Branches      15299    15858     +559     
============================================
+ Hits         102638   108114    +5476     
+ Misses        28908    28658     -250     
- Partials       7956     8267     +311     
Flag Coverage Δ
inttests 27.96% <0.00%> (+3.38%) ⬆️
systests 24.65% <0.00%> (+0.33%) ⬆️
unittests 73.87% <100.00%> (+1.03%) ⬆️

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

Files with missing lines Coverage Δ
.../apache/pulsar/broker/admin/impl/ClustersBase.java 81.83% <100.00%> (+1.34%) ⬆️
.../pulsar/admin/cli/CmdNamespaceIsolationPolicy.java 37.83% <ø> (+1.25%) ⬆️
...mmon/policies/data/NamespaceIsolationDataImpl.java 89.18% <ø> (+3.89%) ⬆️

... and 554 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@grssam
Copy link
Contributor Author

grssam commented Sep 5, 2024

I just noticed. Why is the codecov diff comparing against a 6 months old commit rather than master's head?

…l/ClustersBase.java

Co-authored-by: Zixuan Liu <nodeces@gmail.com>
@grssam
Copy link
Contributor Author

grssam commented Sep 11, 2024

/pulsarbot rerun-failure-checks

@lhotari lhotari merged commit 4f00259 into apache:master Sep 18, 2024
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
…s-isolation-policy set` (apache#23253)

Co-authored-by: Zixuan Liu <nodeces@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-required Your PR changes impact docs and you will update later. ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants