Casey Ching has posted comments on this change. Change subject: Add Kudu test helpers ......................................................................
Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/2855/9/tests/conftest.py File tests/conftest.py: Line 264: """Provides a new DB-API compliant connection to Impala as a pytest fixture. The > Curious - does this mean, in the future, that parallel tests will use the s I'm pretty sure parallelization is at the process level and all processes only run a single thread. Line 267: - get_db_name(): The name of the database to connect to. > Why not just stub this in the base test class, and return None by default? That's an option but it pushes responsibility onto the base class when that responsibility could be handled here. Line 323: cur.execute("DROP DATABASE IF EXISTS %s CASCADE" % db_name) > Nit: Maybe start using .format everywhere for consistency. Both are useful. This version is shorter. More complicated templates can use .format() for clarity. -- 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: 9 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Casey Ching <[email protected]> Gerrit-Reviewer: Casey Ching <[email protected]> Gerrit-Reviewer: Ishaan Joshi <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
