Update namespace & topic admin methods to return nil if unset#1433
Merged
crossoverJie merged 5 commits intoapache:masterfrom Oct 30, 2025
Merged
Update namespace & topic admin methods to return nil if unset#1433crossoverJie merged 5 commits intoapache:masterfrom
crossoverJie merged 5 commits intoapache:masterfrom
Conversation
Member
|
test failure: |
75a181a to
c2e4deb
Compare
c2e4deb to
89e8bd6
Compare
Contributor
Author
|
@lhotari Thanks, fixed up the test issues. I needed to change a bit more in the client package that should be reviewed carefully. |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the behavior of topic and namespace policy getter methods to return nil when policies are not configured at the topic/namespace level, rather than returning default/zero values. This is accomplished by introducing a new GetBodyWithContext method and decodeJSONWithBody helper that detect empty HTTP response bodies.
Key Changes
- Added
GetBodyWithContextanddecodeJSONWithBodyfunctions to REST client for detecting empty response bodies - Updated policy getter methods to return
nilwhen policies are not configured (empty response) - Updated tests to expect
nilvalues instead of zero-value structs when policies are removed or not configured
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pulsaradmin/pkg/rest/client.go | Added GetBodyWithContext method and decodeJSONWithBody helper to detect and return empty response bodies |
| pulsaradmin/pkg/admin/topic.go | Updated policy getter methods and documentation to return nil when policies are not configured |
| pulsaradmin/pkg/admin/topic_test.go | Updated tests to expect nil instead of zero values when policies are not configured |
| pulsaradmin/pkg/admin/namespace.go | Updated namespace policy getter methods and documentation to return nil when policies are not configured |
| pulsaradmin/pkg/admin/namespace_test.go | Added comprehensive tests for namespace policies expecting nil for unconfigured policies |
Comments suppressed due to low confidence (1)
pulsaradmin/pkg/rest/client.go:220
- Line 220 returns
nil, errbuterris undefined at this point if theelse if !decodeblock was entered but neither thefile != nilnor theelsebranch returned. This will cause a compilation error or return an undefined error. Should returnnil, nilinstead.
return nil, err
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
RobertIndie
approved these changes
Oct 30, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1431
Motivation
For the namespace / topic admin methods that return a pointer, explicitly return nil if the method isn't set (API returns empty body)
Modifications
GetBodyWithContextthat returns body & errorVerifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation