Attila Jeges has posted comments on this change. Change subject: IMPALA-3829: OpenSession() logs errors on valid config keys. ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/3662/1/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: Line 620: // the 'impala.doas.user' Hive Server 2 configuration property. > nit: wrap at 90 chars Done PS1, Line 639: Ignore failure (will be logged). > Does this mean any errors will be logged inside SetQueryOption()? Yes, it does. I changed the comment to make it clearer. http://gerrit.cloudera.org:8080/#/c/3662/1/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: Line 29: class TestHS2(HS2TestSuite): > Can you also add tests for the special cases in impala-hs2-server.cc? test_open_session_query_options() method was only meant to test that normal query options are properly set. Special config keys are already tested in other methods. use:database : - test_change_default_database(), - test_change_default_database_case_insensitive() - test_bad_default_database() idle_session_timeout : - test_concurrent_session_mixed_idle_timeout() impala.doas.user : - test_open_session_empty_user() I was thinking about taking a more direct approach and test that the special config settings are properly saved in the session state object. Unfortunately I could not find an RPC call in the HiveServer2 API to retrieve the session state object. I guess, I could add a new call to the API but that seems like an overkill. Line 35: def test_open_sesssion_query_options(self): > May be I missed that somehow but there doesn't seem to be a test for use:da test_open_session_query_options() method was only meant to test that normal query options are properly set. use:database is already tested in the following methods: - test_change_default_database(), - test_change_default_database_case_insensitive() - test_bad_default_database() -- To view, visit http://gerrit.cloudera.org:8080/3662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa9b823abc39ba9809a35a6f0844fa3436f1e025 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-HasComments: Yes
