----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75213/#review226966 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java Lines 182 (patched) <https://reviews.apache.org/r/75213/#comment315256> Consider replacing "1L" with RangerSecurityZone.RANGER_UNZONED_SECURITY_ZONE_ID. security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java Lines 1771 (patched) <https://reviews.apache.org/r/75213/#comment315255> I suggest to mark following methods as private: - getOrCreateDataShare() - getServiceId() - getZoneId() - isValidService() - isValidZone() security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java Lines 1795 (patched) <https://reviews.apache.org/r/75213/#comment315257> "select" may not be a vaid access-type in the given service. I suggest to set defaultAccessTypes to an empty set. security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java Lines 1823 (patched) <https://reviews.apache.org/r/75213/#comment315254> isValidService() and isValidZone() throw exception when the given service or zone is invalid. I suggest these methods to return the ID or throw exception when not found. With this, getServiceId() and getZoneId() can be eliminated. - private Long validateAndGetServiceId(String svcName); - private Long validateAndGetZoneId(String zoneName); - Madhan Neethiraj On Oct. 6, 2024, 8:47 a.m., Ramesh Mani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75213/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2024, 8:47 a.m.) > > > Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, and Radhika > Kundam. > > > Bugs: RANGER-4937 > https://issues.apache.org/jira/browse/RANGER-4937 > > > Repository: ranger > > > Description > ------- > > RANGER-4937: Add a new GDS resource API for adding new resources to a new or > existing DataShare and add it to the DataSet > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java 0b3d91001 > > > Diff: https://reviews.apache.org/r/75213/diff/3/ > > > Testing > ------- > > Add a new GDS resource API for adding new resources to a new or existing > DataShare and add it to the DataSet > > - add a resource to a dataset using existing API "POST service/gds/resource", > with following enhancements: > > Testing done with local vm: > - Create a DataSet with "POST service/gds/dataset". > - Call the enhanced API "/dataset/{id}/resources/{serviceName}" as > following. > > curl -ikv -u admin:Admin123 -X POST -H "Content-Type: application/json" > 'http://localhost:6080/service/gds/dataset/30/resources/cm_hive' -d '[ { > "createdBy": "Admin", "isEnabled": "true", "version": 1, "dataShareId":"-1", > "resource":{ "database":{ "values":["test_db"] }, "table":{ > "values":["sal1","sal2","sal4"] }, "column": { "values": [ "id" ], > "isExcludes": false, "isRecursive": false } }, "name":"test-resource-17", > "accessTypes":["all"] }, { "createdBy": "Admin", "isEnabled": "true", > "version": 1, "dataShareId":"-1", "resource":{ "database":{ > "values":["default"] }, "table":{ > "values":["sal2","emp","tb_1","sal1","sal3","tb_2"] }, "column": { "values": > [ "*" ], "isExcludes": false, "isRecursive": false } }, > "name":"test-resource-18", "accessTypes":["all"] }]' > > curl -ikv -u admin:Admin123 -X POST -H "Content-Type: application/json" > 'http://localhost:6080/service/gds/dataset/30/resources/cm_hive' -d '[ { > "createdBy": "Admin", "isEnabled": "true", "version": 1, "dataShareId":"-1", > "resource":{ "database":{ "values":["test_db3"] }, "table":{ > "values":["sal12","sal22","sal42"] }, "column": { "values": [ "id" ], > "isExcludes": false, "isRecursive": false } }, "name":"test-resource-172", > "accessTypes":["all"] }]' > > > Thanks, > > Ramesh Mani > >
