yuruguo commented on pull request #13090: URL: https://github.com/apache/pulsar/pull/13090#issuecomment-987263389
> @yuruguo - unfortunately, my PR #13133 created merge conflicts for this PR, so you'll need to rebase. > > Based on looking at this PR and at #6428, I think there are a few opportunities for cleanup, but I need to look at the API a little more before I have a fully formed opinion. For example, I think we might be able to add the `@Deprecated` annotation for several enum fields. Here are the ones that I am tentatively considering: `NamespaceOperation.GET_TOPIC`, `NamespaceOperation.ADD_BUNDLE`, `TopicOperation.ADD_BUNDLE_RANGE`, `TopicOperation.GET_BUNDLE_RANGE`, and `TopicOperation.DELETE_BUNDLE_RANGE`. These seem unnecessary, but I might be missing something. It'd probably be worth an email to the dev mailing list before we officially deprecate these fields. > > I also see that we have these unused fields: `TopicOperation.GRANT_PERMISSION`, `TopicOperation.GET_PERMISSION`, and `TopicOperation.REVOKE_PERMISSION`. Perhaps they should be used, though, if we want to support topic level granularity for permissions. The current implementation does not rely on these 3 fields because the `PersistentTopicsBase#internalGrantPermissionsOnTopic` implements logic that I think should reside in an `AuthorizationProvider`. Nice! `TenantOperation` and its enumeration are also useless because we only need to judge whether the `role` is a tenant administrator, so we may also need to take a look it. https://github.com/apache/pulsar/blob/50ab411305fb616439b3313926b7cb9e85ccbec5/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TenantOperation.java#L22-L29 -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
