Copilot commented on code in PR #1479:
URL: https://github.com/apache/pulsar-client-go/pull/1479#discussion_r3071536092
##########
pulsaradmin/pkg/admin/namespace.go:
##########
@@ -747,17 +750,35 @@ type Namespaces interface {
// UpdatePropertiesWithContext updates the properties of a namespace
UpdatePropertiesWithContext(ctx context.Context, namespace
utils.NameSpaceName, properties map[string]string) error
+ // SetProperty sets a namespace property for the given key
+ SetProperty(namespace utils.NameSpaceName, key, value string) error
+
+ // SetPropertyWithContext sets a namespace property for the given key
+ SetPropertyWithContext(ctx context.Context, namespace
utils.NameSpaceName, key, value string) error
+
Review Comment:
The PR description still contains the unfilled template placeholders (e.g.,
`Fixes #<xyz>`, motivation/verification sections, checklist text). Please
update the PR description to reflect the actual motivation, scope,
testing/verification performed, and remove the template boilerplate so
reviewers have the required context.
##########
pulsaradmin/pkg/admin/namespace.go:
##########
@@ -2130,6 +2176,55 @@ func (n *namespaces) RemovePropertiesWithContext(ctx
context.Context, namespace
return n.pulsar.Client.DeleteWithContext(ctx, endpoint)
}
+func (n *namespaces) RemoveProperty(namespace utils.NameSpaceName, key string)
(*string, error) {
+ return n.RemovePropertyWithContext(context.Background(), namespace, key)
+}
+
+func (n *namespaces) RemovePropertyWithContext(
+ ctx context.Context,
+ namespace utils.NameSpaceName,
+ key string,
+) (*string, error) {
+ return n.requestPropertyValueWithContext(ctx, http.MethodDelete,
namespace, key)
+}
+
+func (n *namespaces) requestPropertyValueWithContext(
+ ctx context.Context,
+ method string,
+ namespace utils.NameSpaceName,
+ key string,
+) (*string, error) {
+ endpoint := n.pulsar.endpoint(n.basePath, namespace.String(),
"property", key)
+ resp, err := n.pulsar.Client.MakeRequestWithContext(ctx, method,
endpoint)
+ if err != nil {
+ if adminErr, ok := err.(rest.Error); ok && adminErr.Code ==
http.StatusNotFound {
+ return nil, nil
+ }
+ return nil, err
+ }
+ defer safeRespClose(resp)
+
+ body, err := io.ReadAll(resp.Body)
+ if err != nil {
+ return nil, err
+ }
+ return decodeNamespacePropertyValue(body)
+}
+
+func decodeNamespacePropertyValue(body []byte) (*string, error) {
+ if isUnsetPolicyBody(body) {
+ return nil, nil
+ }
+
Review Comment:
`decodeNamespacePropertyValue` checks `isUnsetPolicyBody(body)` and then
immediately calls `decodeOptionalJSON`, which already treats empty/`null`
bodies as unset. This double-check is redundant (extra string conversions) and
can be simplified by relying on `decodeOptionalJSON` alone and only falling
back to the plain-text parse when it returns a JSON unmarshal error.
##########
pulsaradmin/pkg/admin/namespace.go:
##########
@@ -2130,6 +2176,55 @@ func (n *namespaces) RemovePropertiesWithContext(ctx
context.Context, namespace
return n.pulsar.Client.DeleteWithContext(ctx, endpoint)
}
+func (n *namespaces) RemoveProperty(namespace utils.NameSpaceName, key string)
(*string, error) {
+ return n.RemovePropertyWithContext(context.Background(), namespace, key)
+}
+
+func (n *namespaces) RemovePropertyWithContext(
+ ctx context.Context,
+ namespace utils.NameSpaceName,
+ key string,
+) (*string, error) {
+ return n.requestPropertyValueWithContext(ctx, http.MethodDelete,
namespace, key)
+}
+
+func (n *namespaces) requestPropertyValueWithContext(
+ ctx context.Context,
+ method string,
+ namespace utils.NameSpaceName,
+ key string,
+) (*string, error) {
+ endpoint := n.pulsar.endpoint(n.basePath, namespace.String(),
"property", key)
+ resp, err := n.pulsar.Client.MakeRequestWithContext(ctx, method,
endpoint)
+ if err != nil {
+ if adminErr, ok := err.(rest.Error); ok && adminErr.Code ==
http.StatusNotFound {
+ return nil, nil
+ }
+ return nil, err
+ }
+ defer safeRespClose(resp)
+
+ body, err := io.ReadAll(resp.Body)
+ if err != nil {
+ return nil, err
+ }
+ return decodeNamespacePropertyValue(body)
+}
+
+func decodeNamespacePropertyValue(body []byte) (*string, error) {
+ if isUnsetPolicyBody(body) {
+ return nil, nil
+ }
+
+ value, err := decodeOptionalJSON[string](body)
+ if err == nil {
+ return value, nil
+ }
Review Comment:
`decodeOptionalJSON` / `isUnsetPolicyBody` helpers live in
`topic_policies.go` but are now used by namespace property decoding. Consider
moving these generic decode helpers into a shared file (or renaming) so
namespace APIs don’t implicitly depend on the topic-policies implementation
file for compilation/behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
