Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 )
Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger ...................................................................... Patch Set 16: (7 comments) Hi all, I have addressed most of Laszlo's comments on the patch set 15. Let me know if there is still any additional suggestion. Thank you very much for the help! http://gerrit.cloudera.org:8080/#/c/21160/15//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21160/15//COMMIT_MSG@7 PS15, Line 7: Support running Impala with > maybe rephrase it as something like "Support running Impala with locally bu Thanks Laszlo! You are correct. Building Ranger is not in scope of this patch. I will revise the subject line accordingly in the next patch. http://gerrit.cloudera.org:8080/#/c/21160/15//COMMIT_MSG@14 PS15, Line 14: Impala. : - Switch to the new constructor when instantiating > It would be worth mentioning explicitly that automating Ranger's build from Thanks Laszlo! I will make it clear in the commit message of the next patch. http://gerrit.cloudera.org:8080/#/c/21160/15//COMMIT_MSG@61 PS15, Line 61: > maybe add the Git hash as an absolute reference? Done http://gerrit.cloudera.org:8080/#/c/21160/15/testdata/cluster/ranger/README File testdata/cluster/ranger/README: http://gerrit.cloudera.org:8080/#/c/21160/15/testdata/cluster/ranger/README@3 PS15, Line 3: tip being > add the Git hash here as well Done http://gerrit.cloudera.org:8080/#/c/21160/15/testdata/cluster/ranger/README@4 PS15, Line 4: mpala/commit/1233ac3) > add Git hash? Done http://gerrit.cloudera.org:8080/#/c/21160/15/testdata/cluster/ranger/README@6 PS15, Line 6: patches, we only found the follow > Consider adding an XFAIL (maybe conditionally, it it's practical) for the a Thanks Laszlo! I would like to keep IMPALA-12921_addendum.diff as is since I found this diff file could no longer be cleanly applied against the current tip (https://github.com/apache/impala/commit/bafd190) because Impala's code in some files changed in the diff file has also been modified in the past 3 weeks. The diff file could be cleanly applied against IMPALA-13061 (https://github.com/apache/impala/commit/1233ac3) as I recall. I agree with you that it would be great if the runtime configuration could be determined programmatically. But before we spend time on creating such a patch, it may be better to have a discussion with the Apache Ranger community. Specifically, we don't run Impala's authorization-related tests against each incoming Apache Ranger patch set. There is no way for us (Apache Impala developers) to know what authorization-related tests could be failed (or fixed) by a new Apache Ranger patch for whatever reason. Actually, before RANGER-4771 is resolved, almost all of the authorization-related tests will fail as explained at https://gerrit.cloudera.org/c/21160/15/testdata/cluster/ranger/README#19. Issues like this should be resolved on the Ranger side first. http://gerrit.cloudera.org:8080/#/c/21160/15/testdata/cluster/ranger/README@18 PS15, Line 18: RANGER-4771 > Maybe add the Git hash for an absolute reference? Done -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: John Sherman <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Sun, 09 Jun 2024 02:40:46 +0000 Gerrit-HasComments: Yes
