[fix][broker] Return non-null persistence policy for namespace #14158
Conversation
rdhabalia
requested review from
merlimat,
sijie,
jiazhai,
eolivelli,
315157973,
codelipenghui and
congbobo184
|
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs? |
1 similar comment
|
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs? |
|
this change only impacts api-get call code-path and it will not change set-policy with default values. |
Agree, with this change, users will not able to know what is the namespace settings. We should apply the same behavior, for policies, if the topic/namespace don't have a policy, just provide a response to tell users no policy for this topic/namespace, if users want to check which policy will be applied, they can add --applied`. |
|
I will move this one to milestone 2.11 first. |
this is incorrect, this PR only sets default persistence policy if the value is not configured and right now it's returning null which is confusing for user. we don't need any additional control flag to show null value to user. and regrading other policies , that can be handled in separate PR and we don't want to mix everything in this one. @codelipenghui this PR has minor change which doesn't break any change then why should we change the release for this PR and why can't we merge to 2.10 ? |
|
On a note completely unrelated to the content of this PR, we're able to merge this PR without a test passing because it is not "required". I am working on fixing this by updating the
|
|
The pr had no activity for 30 days, mark with Stale label. |
|
Reopened to trigger CI |
|
The pr had no activity for 30 days, mark with Stale label. |
|
The pr had no activity for 30 days, mark with Stale label. |
tisonkun
changed the title
github-actions
bot
added
doc-not-needed
Codecov Report
@@ Coverage Diff @@
## master #14158 +/- ##
=============================================
- Coverage 46.34% 34.84% -11.51%
+ Complexity 10394 7480 -2914
=============================================
Files 703 703
Lines 68838 68840 +2
Branches 7379 7378 -1
=============================================
- Hits 31905 23988 -7917
- Misses 33324 41645 +8321
+ Partials 3609 3207 -402
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
It seems the codebase evolves a lot and the added test Close as stale. @rdhabalia you may pick up this patch and rebase on the current master to pass the related tests and resubmit it again. |
Motivation
Right now, if namespace is not configured with persistence policy then broker considers default persistence-policy while serving the topic but it returns null value in get-persistence admin API
Modification
Return default persistence policy if namespace persistence policy is not configured.
doc-not-needed