David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14313 )

Change subject: Fix --webserver_interface for remote cluster tests
......................................................................


Patch Set 3:

Sorry that I'm coming to this late, but I don't think this will address testing 
against a remote cluster. Another issue is that when we run these tests against 
a remote clusters, the local minicluster is NOT required to be running, which 
means L124 and L125 from impala_test_suite.py will break:

  METRICS_URL = 'http://localhost:25000/metrics?json'
  VARZ_URL = 'http://localhost:25000/varz?json'

...since there's nothing listening at localhost:25000.

  tests/common/impala_test_suite.py:339: in get_var_current_val
      varz = self.get_debug_page(VARZ_URL)
  tests/common/impala_test_suite.py:332: in get_debug_page
      response = requests.get(page_url)
  infra/python/env/local/lib/python2.7/site-packages/requests/api.py:69: in get
      return request('get', url, params=params, **kwargs)
  infra/python/env/local/lib/python2.7/site-packages/requests/api.py:50: in 
request
      response = session.request(method=method, url=url, **kwargs)
  infra/python/env/local/lib/python2.7/site-packages/requests/sessions.py:465: 
in request
      resp = self.send(prep, **send_kwargs)
  infra/python/env/local/lib/python2.7/site-packages/requests/sessions.py:573: 
in send
      r = adapter.send(request, **kwargs)
  infra/python/env/local/lib/python2.7/site-packages/requests/adapters.py:415: 
in send
      raise ConnectionError(err, request=request)
  E   ConnectionError: ('Connection aborted.', error(111, 'Connection refused'))

Basically, hard-coding "localhost" anywhere in any part of the test framework 
is always a bad idea. The better solution is to use whatever pytest thinks the 
"impalad" value is -- although the problem there is that it's variably either a 
comma-delimited string of multiple hosts + port 21000 (from conftest.py, L46):

  DEFAULT_IMPALADS = "localhost:21000,localhost:21001,localhost:21002"

or else simply "hostname:21000" if specified on the command line with 
"--impalad". So the hardcoded port numbers are an issue as well. :-(

I just ran a quick test, and this approach, as ugly as it looks, works. I'll 
open a new patch soon.

  impalad_hostname = pytest.config.option.impalad.split(',')[0].split(':')[0]
  METRICS_URL = 'http://{0}:25000/metrics?json'.format(impalad_hostname)
  VARZ_URL = 'http://{0}:25000/varz?json'.format(impalad_hostname)


--
To view, visit http://gerrit.cloudera.org:8080/14313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5b8adefe4e2bb8f7013d9af70fd3e5dfd7ee18f
Gerrit-Change-Number: 14313
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Fri, 27 Sep 2019 22:10:23 +0000
Gerrit-HasComments: No

Reply via email to