Skip to content

Commit

Permalink
Fix: clear null values from options dictionary
Browse files Browse the repository at this point in the history
  • Loading branch information
arikfr committed Jun 13, 2017
1 parent ef2eaf1 commit a824647
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 6 deletions.
7 changes: 5 additions & 2 deletions redash/handlers/data_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
require_permission, view_only)
from redash.query_runner import (get_configuration_schema_for_query_runner_type,
query_runners)
from redash.utils import filter_none
from redash.utils.configuration import ConfigurationContainer, ValidationError


Expand All @@ -35,7 +36,7 @@ def post(self, data_source_id):
abort(400)
try:
data_source.options.set_schema(schema)
data_source.options.update(req['options'])
data_source.options.update(filter_none(req['options']))
except ValidationError:
abort(400)

Expand Down Expand Up @@ -88,7 +89,9 @@ def post(self):
if schema is None:
abort(400)

config = ConfigurationContainer(req['options'], schema)
config = ConfigurationContainer(filter_none(req['options']), schema)
# from IPython import embed
# embed()
if not config.is_valid():
abort(400)

Expand Down
6 changes: 5 additions & 1 deletion redash/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import pytz
import pystache

from funcy import distinct
from funcy import distinct, select_values
from sqlalchemy.orm.query import Query

from .human_time import parse_human_time
Expand Down Expand Up @@ -165,3 +165,7 @@ def base_url(org):
return "https://{}/{}".format(settings.HOST, org.slug)

return settings.HOST


def filter_none(d):
return select_values(lambda v: v is not None, d)
4 changes: 2 additions & 2 deletions tests/handlers/test_data_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def setUp(self):
def test_returns_400_when_configuration_invalid(self):
admin = self.factory.create_admin()
rv = self.make_request('post', self.path,
data={'name': 'DS 1', 'type': 'pg', 'options': '{}'}, user=admin)
data={'name': 'DS 1', 'type': 'pg', 'options': {}}, user=admin)

self.assertEqual(rv.status_code, 400)

Expand Down Expand Up @@ -94,7 +94,7 @@ def test_returns_400_when_missing_fields(self):
def test_returns_400_when_configuration_invalid(self):
admin = self.factory.create_admin()
rv = self.make_request('post', '/api/data_sources',
data={'name': 'DS 1', 'type': 'pg', 'options': '{}'}, user=admin)
data={'name': 'DS 1', 'type': 'pg', 'options': {}}, user=admin)

self.assertEqual(rv.status_code, 400)

Expand Down
14 changes: 13 additions & 1 deletion tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from redash.utils import build_url, collect_query_parameters, collect_parameters_from_request
from collections import namedtuple
from unittest import TestCase

from redash.utils import (build_url, collect_parameters_from_request,
collect_query_parameters, filter_none)

DummyRequest = namedtuple('DummyRequest', ['host', 'scheme'])


Expand Down Expand Up @@ -49,3 +51,13 @@ def test_ignores_non_prefixed_values(self):

def test_takes_prefixed_values(self):
self.assertDictEqual({'test': 1, 'something_else': 'test'}, collect_parameters_from_request({'p_test': 1, 'p_something_else': 'test'}))


class TestSkipNones(TestCase):
def test_skips_nones(self):
d = {
'a': 1,
'b': None
}

self.assertDictEqual(filter_none(d), {'a': 1})

0 comments on commit a824647

Please sign in to comment.