[ https://issues.apache.org/jira/browse/SENTRY-2493?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16762165#comment-16762165 ]
Arjun Mishra commented on SENTRY-2493: -------------------------------------- [~kkalyan] My comments for your code review {code} Comments 1. NotificationProcessor: Change this log message "LOGGER.info("Empty paths, not updating sentry store");" to include the authzObj as well 2. <nit> QueryParamBuilder: Why not change this "if (paths == null || paths.size() == 0) {" to using isEmpty like you've done in other places 3. <nit> TestSentryStore: Comment "//Try to mapping with null paths" is not grammatically right. Also comment needs to describe the test. Its not very clear at the moment 4. TestSentryStore: Why would we pass null or Empty paths to deleteAuthzPathsMapping? You have handled is null or is empty in NotificationsProcess. Why do we want to delete the authz objects if paths is null or empty? {code} > Sentry store api's for path mapping should handle empty/null paths. > ------------------------------------------------------------------- > > Key: SENTRY-2493 > URL: https://issues.apache.org/jira/browse/SENTRY-2493 > Project: Sentry > Issue Type: Bug > Affects Versions: 2.2.0 > Reporter: kalyan kumar kalvagadda > Assignee: kalyan kumar kalvagadda > Priority: Major > Fix For: 2.2.0 > > Attachments: SENTRY-2493.001.patch > > > Current API's for adding and deleting path mapping throw exceptions when > paths provided are either empty/null. API's should handle it gracefully. -- This message was sent by Atlassian JIRA (v7.6.3#76005)