Fredy Wijaya 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 3: Code-Review+1 (3 comments) Carry Csaba's +1. 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: his pa > Typo? Oops. Done. 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 It's also used in catalog-util-test.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: assert "catalog_version" in obj_dump > nit: this does not look as descriptive as 'assert "CatalogException" in obj Done -- 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: 3 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 15:36:03 +0000 Gerrit-HasComments: Yes