impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brown (Code Review)" <>
Subject [Impala-CR](cdh5-trunk) Add Kudu test helpers
Date Thu, 28 Apr 2016 23:23:27 GMT
Michael Brown has posted comments on this change.

Change subject: Add Kudu test helpers

Patch Set 1:


I did some basic testing of patch set 4 and the LD_LIBRARY_PATH manipulation stuff seems to
be working on my machine.
File tests/

Line 242: 
> The main difference is the db api/impyla client. I'd rather use that interf
Do you mind updating both the unique_database's and this fixture's docstrings to reflect the
differences then? You're almost there in the "conn" docstring case, but go ahead and mention
the beeswax and hs2 support explicitly. Thanks.

Line 260: def conn(request):
> What do you suggest? I think it's fine as-is. A generic "conn" would be an 
impala_connection ?

"conn" as a name to me is more suited, for example, as the temporary name for a connection
when iterating through a sequence of connections. I'd prefer the fixtures to be a bit more

Line 272:   db_name = __call_cls_method_if_exists(request.cls, "get_db_name")
        :   use_unique_conn = __call_cls_method_if_exists(request.cls, "auto_create_db")
> That would be very verbose. Being explicit might be helpful though. I ended
Understood. Thanks for documenting the proper use.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e5e22b38d5bd09a36238e66a69aa42d1a941de7
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <>
Gerrit-Reviewer: Casey Ching <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-HasComments: Yes

View raw message