Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13933 )

Change subject: IMPALA-8627: Enable catalog-v2 in tests
......................................................................


Patch Set 11:

(4 comments)

Addressed Tim's comments and suggestions.

http://gerrit.cloudera.org:8080/#/c/13933/11/tests/hs2/hs2_test_suite.py
File tests/hs2/hs2_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/13933/11/tests/hs2/hs2_test_suite.py@74
PS11, Line 74:     def add_session(self, cluster_properties):
> I think we can avoid all of the duplication with the above add session func
Thanks for sharing your the code snippet. Incorporated into the patch.


http://gerrit.cloudera.org:8080/#/c/13933/11/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/13933/11/tests/hs2/test_hs2.py@440
PS11, Line 440:   @needs_session_cluster_properties()
> If we're defining a new decorator anyway, could we pass in unique_database
Done


http://gerrit.cloudera.org:8080/#/c/13933/11/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/13933/11/tests/query_test/test_observability.py@318
PS11, Line 318:         r'CatalogFetch.Config.Misses|CatalogFetch.Config.Hits',
> Is it guaranteed that we always get one or the other?
yes, according to 
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java#L516
Its always either a hit or miss and hence one of the counter will be added. I 
ran this test multiple times which generates a miss event first and then later 
on hit event subsequently.


http://gerrit.cloudera.org:8080/#/c/13933/11/tests/query_test/test_observability.py@340
PS11, Line 340:     self.__verify_profile_event_sequence(load_event_regexes, 
runtime_profile)
> Do we need to test that the above regexes are in sequential lines, or just
Yeah, currently, they are in this order. We can change it to verify it they are 
present in different order by sort them out but it needed some more plumbing. I 
would like to defer it to later when its really needed.



--
To view, visit http://gerrit.cloudera.org:8080/13933
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddbde666de2b780c0e40df716a9dfe54524e092d
Gerrit-Change-Number: 13933
Gerrit-PatchSet: 11
Gerrit-Owner: Vihang Karajgaonkar <vih...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 19:05:35 +0000
Gerrit-HasComments: Yes

Reply via email to