Michael Brown has posted comments on this change.

Change subject: IMPALA-1683: Allow REFRESH on a single partition
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3385/9/tests/metadata/test_refresh_partition.py
File tests/metadata/test_refresh_partition.py:

PS9, Line 27: class ImpalaTableWrapper(object):
> So, if we were to make something like this class generally available, it sh
My take is that while this could be useful for some test code, it's a bit 
superfluous here for these reasons:

1. Tables created with ImpalaTableWrapper are using unique_database in all 
instances. unique_database will handle the "unique namespace" for the created 
table, and unique_database will DROP CASCADE, which handles cleanup, even 
during exception, thanks to the fixture's finalizer.

2. Only 1 table is being created for each test anyway. Since they are using 
unique_database already, they have exclusion from each other already.

3. The context usages of ImpalaTableWrapper are for the full durations of 
tests. I.e., the table is created at the very beginning of the test proper and 
is expected to persist until the end. In that sense, it "bumpers" 
unique_database.

If instead a test needed to manage many tables at a time, or deal with separate 
contexts of tables' existing, then I could see a stronger reason to have this.

Do you anticipate such tests?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bf5dd630f2e175be5800187bbf9337c91f82bd8
Gerrit-PatchSet: 9
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Michael Brown <[email protected]>
Gerrit-HasComments: Yes

Reply via email to