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

Reply via email to