Skip to content

Commit

Permalink
Minor code smell cleanup. (#2820)
Browse files Browse the repository at this point in the history
* Remove unused parse_db_url function.

* Fix tests to not show command line warnings anymore.

* Minor code smell cleanup.

Removing unneeded imports fixing PEP8 issues.
  • Loading branch information
jezdez authored and arikfr committed Sep 16, 2018
1 parent 5c9852b commit b1f5d60
Show file tree
Hide file tree
Showing 20 changed files with 46 additions and 97 deletions.
5 changes: 2 additions & 3 deletions redash/__init__.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import os
import sys
import logging
import urlparse
import urllib
import redis
from flask import Flask, safe_join
from flask import Flask
from flask_sslify import SSLify
from werkzeug.contrib.fixers import ProxyFix
from werkzeug.routing import BaseConverter, ValidationError
from werkzeug.routing import BaseConverter
from statsd import StatsClient
from flask_mail import Mail
from flask_limiter import Limiter
Expand Down
1 change: 0 additions & 1 deletion redash/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from flask_admin.base import MenuLink
from flask_admin.contrib.sqla import ModelView
from flask_admin.contrib.sqla.form import AdminModelConverter
from flask_admin.form.widgets import DateTimePickerWidget
from wtforms import fields
from wtforms.widgets import TextInput

Expand Down
4 changes: 2 additions & 2 deletions redash/cli/data_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
from sqlalchemy.orm.exc import NoResultFound

from redash import models
from redash.query_runner import query_runners
from redash.query_runner import get_configuration_schema_for_query_runner_type
from redash.query_runner import (get_configuration_schema_for_query_runner_type,
query_runners)
from redash.utils.configuration import ConfigurationContainer

manager = AppGroup(help="Data sources management commands.")
Expand Down
2 changes: 0 additions & 2 deletions redash/destinations/__init__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import logging
import json

from redash import settings

logger = logging.getLogger(__name__)

__all__ = [
Expand Down
1 change: 0 additions & 1 deletion redash/handlers/authentication.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import hashlib
import logging

from flask import abort, flash, redirect, render_template, request, url_for
Expand Down
8 changes: 4 additions & 4 deletions redash/handlers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
from flask_restful import Resource, abort
from redash import settings
from redash.authentication import current_org
from redash.models import ApiUser, db
from redash.models import db
from redash.tasks import record_event as record_event_task
from redash.utils import json_dumps
from sqlalchemy.orm.exc import NoResultFound
from sqlalchemy import cast, select, String
from sqlalchemy import cast
from sqlalchemy.dialects import postgresql
from sqlalchemy_utils import sort_query

Expand Down Expand Up @@ -130,7 +130,7 @@ def filter_by_tags(result_set, column):
if request.args.getlist('tags'):
tags = request.args.getlist('tags')
result_set = result_set.filter(cast(column, postgresql.ARRAY(db.Text)).contains(tags))

return result_set


Expand All @@ -146,4 +146,4 @@ def order_results(results, default_order, orders_whitelist):
selected_order = orders_whitelist.get(order, default_order)
# The query may already have an ORDER BY statement attached
# so we clear it here and apply the selected order
return sort_query(results.order_by(None), selected_order)
return sort_query(results.order_by(None), selected_order)
17 changes: 10 additions & 7 deletions redash/handlers/favorites.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def get(self):

return response


class QueryFavoriteResource(BaseResource):
def post(self, query_id):
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
Expand All @@ -52,18 +52,21 @@ def post(self, query_id):
else:
raise e


self.record_event({
'action': 'favorite',
'object_id': query.id,
'object_type': 'query'
})

def delete(self, query_id):
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
require_access(query.groups, self.current_user, view_only)

models.Favorite.query.filter(models.Favorite.object==query, models.Favorite.user==self.current_user).delete()
models.Favorite.query.filter(
models.Favorite.object_id == query_id,
models.Favorite.object_type == u'Query',
models.Favorite.user==self.current_user,
).delete()
models.db.session.commit()

self.record_event({
Expand Down Expand Up @@ -101,7 +104,7 @@ def get(self):

return response


class DashboardFavoriteResource(BaseResource):
def post(self, object_id):
dashboard = get_object_or_404(models.Dashboard.get_by_slug_and_org, object_id, self.current_org)
Expand All @@ -121,7 +124,7 @@ def post(self, object_id):
'object_id': dashboard.id,
'object_type': 'dashboard'
})

def delete(self, object_id):
dashboard = get_object_or_404(models.Dashboard.get_by_slug_and_org, object_id, self.current_org)
models.Favorite.query.filter(models.Favorite.object==dashboard, models.Favorite.user==self.current_user).delete()
Expand All @@ -130,4 +133,4 @@ def delete(self, object_id):
'action': 'unfavorite',
'object_id': dashboard.id,
'object_type': 'dashboard'
})
})
5 changes: 1 addition & 4 deletions redash/handlers/organization.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import json

from flask import request
from flask_login import current_user, login_required

from redash import models
from redash.handlers import routes
from redash.handlers.base import json_response, org_scoped_rule
from redash.authentication import current_org
from redash.permissions import require_admin


@routes.route(org_scoped_rule('/api/organization/status'), methods=['GET'])
Expand Down
3 changes: 1 addition & 2 deletions redash/handlers/visualizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
from redash import models
from redash.handlers.base import BaseResource, get_object_or_404
from redash.serializers import serialize_visualization
from redash.permissions import (require_admin_or_owner,
require_object_modify_permission,
from redash.permissions import (require_object_modify_permission,
require_permission)


Expand Down
10 changes: 6 additions & 4 deletions redash/models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import six
import cStringIO
import csv
import datetime
Expand All @@ -8,22 +7,25 @@
import json
import logging
import time
from functools import reduce

import six
import xlsxwriter
from flask import current_app as app, url_for
from flask_login import AnonymousUserMixin, UserMixin
from flask_sqlalchemy import SQLAlchemy, BaseQuery
from passlib.apps import custom_app_context as pwd_context

from redash import settings, redis_connection, utils
from redash.destinations import (get_configuration_schema_for_destination_type,
get_destination)
from redash.metrics import database # noqa: F401
from redash.permissions import has_access, view_only
from redash.query_runner import (get_configuration_schema_for_query_runner_type,
get_query_runner)
from redash.utils import generate_token, json_dumps
from redash.utils.configuration import ConfigurationContainer
from redash.settings.organization import settings as org_settings

from sqlalchemy import distinct, or_, and_, UniqueConstraint
from sqlalchemy.dialects import postgresql
from sqlalchemy.event import listens_for
Expand All @@ -34,7 +36,6 @@
from sqlalchemy.orm.exc import NoResultFound # noqa: F401
from sqlalchemy.types import TypeDecorator
from sqlalchemy.orm.attributes import flag_modified
from functools import reduce
from sqlalchemy import func
from sqlalchemy_searchable import SearchQueryMixin, make_searchable, vectorizer
from sqlalchemy_utils import generic_relationship, EmailType
Expand Down Expand Up @@ -93,6 +94,7 @@ def get(self, query_id):

return timestamp


scheduled_queries_executions = ScheduledQueriesExecutions()

# AccessPermission and Change use a 'generic foreign key' approach to refer to
Expand Down Expand Up @@ -763,7 +765,7 @@ def get_latest(cls, data_source, query, max_age=0):
db.func.timezone('utc', QueryResult.retrieved_at) +
datetime.timedelta(seconds=max_age) >=
db.func.timezone('utc', db.func.now())
).order_by(QueryResult.retrieved_at.desc())
).order_by(QueryResult.retrieved_at.desc())

return q.first()

Expand Down
2 changes: 0 additions & 2 deletions redash/query_runner/__init__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import sys
import logging
import json

from collections import OrderedDict
from redash import settings

logger = logging.getLogger(__name__)
Expand Down
2 changes: 1 addition & 1 deletion redash/settings/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os
from funcy import distinct, remove

from .helpers import parse_db_url, fix_assets_path, array_from_string, parse_boolean, int_or_none, set_from_string
from .helpers import fix_assets_path, array_from_string, parse_boolean, int_or_none, set_from_string


def all_settings():
Expand Down
19 changes: 0 additions & 19 deletions redash/settings/helpers.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,5 @@
import json
import os
import urlparse
import urllib


def parse_db_url(url):
url_parts = urlparse.urlparse(url)
connection = {'threadlocals': True}

if url_parts.hostname and not url_parts.path:
connection['name'] = url_parts.hostname
else:
connection['name'] = url_parts.path[1:]
connection['host'] = url_parts.hostname
connection['port'] = url_parts.port
connection['user'] = url_parts.username
# Passwords might be quoted with special characters
connection['password'] = url_parts.password and urllib.unquote(url_parts.password)

return connection


def fix_assets_path(path):
Expand Down
1 change: 1 addition & 0 deletions redash/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def __call__(self, *args, **kwargs):
with current_app.app_context():
return TaskBase.__call__(self, *args, **kwargs)


celery.Task = ContextTask


Expand Down
2 changes: 1 addition & 1 deletion tests/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os
import logging
import datetime
import json
import logging
from unittest import TestCase
from contextlib import contextmanager

Expand Down
6 changes: 3 additions & 3 deletions tests/handlers/test_favorites.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def test_favorite(self):

rv = self.make_request('get', '/api/queries/{}'.format(query.id))
self.assertEqual(rv.json['is_favorite'], True)

def test_duplicate_favorite(self):
query = self.factory.create_query()

Expand All @@ -21,7 +21,6 @@ def test_duplicate_favorite(self):
rv = self.make_request('post', '/api/queries/{}/favorite'.format(query.id))
self.assertEqual(rv.status_code, 200)


def test_unfavorite(self):
query = self.factory.create_query()
rv = self.make_request('post', '/api/queries/{}/favorite'.format(query.id))
Expand All @@ -31,7 +30,8 @@ def test_unfavorite(self):
rv = self.make_request('get', '/api/queries/{}'.format(query.id))
self.assertEqual(rv.json['is_favorite'], False)


class TestQueryFavoriteListResource(BaseTestCase):
def test_get_favorites(self):
rv = self.make_request('get', '/api/queries/favorites')
self.assertEqual(rv.status_code, 200)
self.assertEqual(rv.status_code, 200)
2 changes: 1 addition & 1 deletion tests/handlers/test_query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_access_with_query_api_key_without_query_result_id(self):

rv = self.make_request('get', '/api/queries/{}/results.json?api_key={}'.format(query.id, query.api_key), user=False)
self.assertEquals(rv.status_code, 200)

def test_query_api_key_and_different_query_result(self):
ds = self.factory.create_data_source(group=self.factory.org.default_group, view_only=False)
query = self.factory.create_query(query_text="SELECT 8")
Expand Down
8 changes: 4 additions & 4 deletions tests/models/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,21 @@ def test_finds_users(self):
self.assertIn(user2, users)

def test_finds_users_case_insensitive(self):
user = self.factory.create_user(email='[email protected]')
user = self.factory.create_user(email=u'[email protected]')

users = User.find_by_email('[email protected]')
users = User.find_by_email(u'[email protected]')
self.assertIn(user, users)


class TestUserGetByEmailAndOrg(BaseTestCase):
def test_get_user_by_email_and_org(self):
user = self.factory.create_user(email='[email protected]')
user = self.factory.create_user(email=u'[email protected]')

found_user = User.get_by_email_and_org(user.email, user.org)
self.assertEqual(user, found_user)

def test_get_user_by_email_and_org_case_insensitive(self):
user = self.factory.create_user(email='[email protected]')
user = self.factory.create_user(email=u'[email protected]')

found_user = User.get_by_email_and_org("[email protected]", user.org)
self.assertEqual(user, found_user)
Expand Down
Loading

0 comments on commit b1f5d60

Please sign in to comment.