Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add https support for Druid #4480

Merged
merged 2 commits into from Feb 28, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions run_specific_test.sh
@@ -1,6 +1,7 @@
#!/usr/bin/env bash
echo $DB
rm -f .coverage
export PYTHONPATH=./
export SUPERSET_CONFIG=tests.superset_test_config
set -e
superset/bin/superset version -v
Expand Down
1 change: 1 addition & 0 deletions run_tests.sh
Expand Up @@ -4,6 +4,7 @@ rm ~/.superset/unittests.db
rm ~/.superset/celerydb.sqlite
rm ~/.superset/celery_results.sqlite
rm -f .coverage
export PYTHONPATH=./
export SUPERSET_CONFIG=tests.superset_test_config
set -e
superset/bin/superset db upgrade
Expand Down
23 changes: 14 additions & 9 deletions superset/connectors/druid/models.py
Expand Up @@ -107,24 +107,29 @@ def data(self):
'backend': 'druid',
}

@staticmethod
def get_base_url(host, port):
if not host.startswith('http'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unlikely, but this might cause problems if the hostname starts with http, eg, http-server.example.com. I think it's better to use a regex here:

if not re.match('http(s)?://', host):
    host = 'http://' + host

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call

host = 'http://' + host
return '{0}:{1}'.format(host, port)

def get_base_coordinator_url(self):
base_url = self.get_base_url(
self.coordinator_host, self.coordinator_port)
return '{base_url}/{self.coordinator_endpoint}'.format(**locals())

def get_pydruid_client(self):
cli = PyDruid(
'http://{0}:{1}/'.format(self.broker_host, self.broker_port),
self.get_base_url(self.broker_host, self.broker_port),
self.broker_endpoint)
return cli

def get_datasources(self):
endpoint = (
'http://{obj.coordinator_host}:{obj.coordinator_port}/'
'{obj.coordinator_endpoint}/datasources'
).format(obj=self)

endpoint = self.get_base_coordinator_url() + '/datasources'
return json.loads(requests.get(endpoint).text)

def get_druid_version(self):
endpoint = (
'http://{obj.coordinator_host}:{obj.coordinator_port}/status'
).format(obj=self)
endpoint = self.get_base_coordinator_url() + '/status'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mistercrunch this is broken, it should use get_base_url() instead.

return json.loads(requests.get(endpoint).text)['version']

def refresh_datasources(
Expand Down
33 changes: 26 additions & 7 deletions tests/druid_tests.py
Expand Up @@ -77,6 +77,16 @@ class DruidTests(SupersetTestCase):
def __init__(self, *args, **kwargs):
super(DruidTests, self).__init__(*args, **kwargs)

def get_test_cluster_obj(self):
return DruidCluster(
cluster_name='test_cluster',
coordinator_host='localhost',
coordinator_endpoint='druid/coordinator/v1/metadata',
coordinator_port=7979,
broker_host='localhost',
broker_port=7980,
metadata_last_refreshed=datetime.now())

@patch('superset.connectors.druid.models.PyDruid')
def test_client(self, PyDruid):
self.login(username='admin')
Expand All @@ -95,13 +105,7 @@ def test_client(self, PyDruid):
db.session.delete(cluster)
db.session.commit()

cluster = DruidCluster(
cluster_name='test_cluster',
coordinator_host='localhost',
coordinator_port=7979,
broker_host='localhost',
broker_port=7980,
metadata_last_refreshed=datetime.now())
cluster = self.get_test_cluster_obj()

db.session.add(cluster)
cluster.get_datasources = PickableMock(return_value=['test_datasource'])
Expand Down Expand Up @@ -323,6 +327,21 @@ def test_sync_druid_perm(self, PyDruid):
permission=permission, view_menu=view_menu).first()
assert pv is not None

def test_urls(self):
cluster = self.get_test_cluster_obj()
self.assertEquals(
cluster.get_base_url('localhost', '9999'), 'http://localhost:9999')
self.assertEquals(
cluster.get_base_url('http://localhost', '9999'),
'http://localhost:9999')
self.assertEquals(
cluster.get_base_url('https://localhost', '9999'),
'https://localhost:9999')

self.assertEquals(
cluster.get_base_coordinator_url(),
'http://localhost:7979/druid/coordinator/v1/metadata')


if __name__ == '__main__':
unittest.main()