Michael Brown has posted comments on this change. Change subject: Add Kudu test helpers ......................................................................
Patch Set 1: (3 comments) I did some basic testing of patch set 4 and the LD_LIBRARY_PATH manipulation stuff seems to be working on my machine. http://gerrit.cloudera.org:8080/#/c/2855/1/tests/conftest.py File tests/conftest.py: 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 explicit. 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 http://gerrit.cloudera.org:8080/2855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e5e22b38d5bd09a36238e66a69aa42d1a941de7 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Casey Ching <[email protected]> Gerrit-Reviewer: Casey Ching <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
