David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/12639 )
Change subject: IMPALA-7995: part 1: fixes for e2e dockerised impala tests ...................................................................... Patch Set 17: (1 comment) http://gerrit.cloudera.org:8080/#/c/12639/17/tests/common/impala_cluster.py File tests/common/impala_cluster.py: http://gerrit.cloudera.org:8080/#/c/12639/17/tests/common/impala_cluster.py@82 PS17, Line 82: def get_e2e_test_cluster(cls): I'm a little confused about the necessity for this additional classmethod that returns an instance. I'm probably missing something obvious. But why not, e.g., just use tests.common.environ.docker_network as the default value in, say, the __init__ method signature (or something along those lines)? It looks like it also defaults to None. -- To view, visit http://gerrit.cloudera.org:8080/12639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee86cbd2c4631a014af1e8cef8e1cd523a812755 Gerrit-Change-Number: 12639 Gerrit-PatchSet: 17 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 12 Apr 2019 22:20:05 +0000 Gerrit-HasComments: Yes