Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11721 )
Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a privilege ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Just a few minor issues, lgtm otherwise. http://gerrit.cloudera.org:8080/#/c/11721/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11721/2//COMMIT_MSG@12 PS2, Line 12: types. Typo? http://gerrit.cloudera.org:8080/#/c/11721/2/be/src/catalog/catalog-util.h File be/src/catalog/catalog-util.h: http://gerrit.cloudera.org:8080/#/c/11721/2/be/src/catalog/catalog-util.h@104 PS2, Line 104: /// Populates a TPrivilegeLevel::type based on the given object name string. : Status TPrivilegeLevelFromObjectName(const std::string& object_name, : TPrivilegeLevel::type* privilege_level); Does this have to be in the header? It is only used in catalog-util.cc http://gerrit.cloudera.org:8080/#/c/11721/2/tests/authorization/test_authorization.py File tests/authorization/test_authorization.py: http://gerrit.cloudera.org:8080/#/c/11721/2/tests/authorization/test_authorization.py@468 PS2, Line 468: service.extract_catalog_object_version(obj_dump) nit: this does not look as descriptive as 'assert "CatalogException" in obj_dump' - maybe 'assert "CatalogException" not in obj_dump' or 'assert "catalog_version" in obj_dump' would clearer. -- To view, visit http://gerrit.cloudera.org:8080/11721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7 Gerrit-Change-Number: 11721 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Thu, 18 Oct 2018 12:45:02 +0000 Gerrit-HasComments: Yes