Skip to content

Commit

Permalink
[#642] Use single quotes where possible
Browse files Browse the repository at this point in the history
  • Loading branch information
@domoritz
domoritz committed Apr 3, 2013
1 parent d76c657 commit 705c39e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 42 deletions.
30 changes: 14 additions & 16 deletions ckanext/datastore/db.py
View file
Expand Up @@ -65,7 +65,7 @@ def _pluck(field, arr):


def _get_list(input, strip=True):
"""Transforms a string or list to a list"""
'''Transforms a string or list to a list'''
if input is None:
return
if input == '':
Expand Down Expand Up @@ -114,7 +114,7 @@ def _validate_int(i, field_name, non_negative=False):


def _get_engine(context, data_dict):
'Get either read or write engine.'
'''Get either read or write engine.'''
connection_url = data_dict['connection_url']
engine = _engines.get(connection_url)

Expand Down Expand Up @@ -181,10 +181,8 @@ def _get_type(context, oid):


def _rename_json_field(data_dict):
'''
rename json type to a corresponding type for the datastore since
pre 9.2 postgres versions do not support native json
'''
'''Rename json type to a corresponding type for the datastore since
pre 9.2 postgres versions do not support native json'''
return _rename_field(data_dict, 'json', 'nested')


Expand All @@ -201,7 +199,8 @@ def _rename_field(data_dict, term, replace):


def _guess_type(field):
'Simple guess type of field, only allowed are integer, numeric and text'
'''Simple guess type of field, only allowed are
integer, numeric and text'''
data_types = set([int, float])
if isinstance(field, (dict, list)):
return 'nested'
Expand Down Expand Up @@ -260,7 +259,7 @@ def json_get_values(obj, current_list=None):


def check_fields(context, fields):
'Check if field types are valid.'
'''Check if field types are valid.'''
for field in fields:
if field.get('type') and not _is_valid_pg_type(context, field['type']):
raise ValidationError({
Expand Down Expand Up @@ -289,7 +288,7 @@ def convert(data, type_name):


def create_table(context, data_dict):
'Create table from combination of fields and first row of data.'
'''Create table from combination of fields and first row of data.'''

datastore_fields = [
{'id': '_id', 'type': 'serial primary key'},
Expand Down Expand Up @@ -339,7 +338,7 @@ def create_table(context, data_dict):


def _get_aliases(context, data_dict):
''' Get a list of aliases for a resource. '''
'''Get a list of aliases for a resource.'''
res_id = data_dict['resource_id']
alias_sql = sqlalchemy.text(
u'SELECT name FROM "_table_metadata" WHERE alias_of = :id')
Expand All @@ -348,8 +347,8 @@ def _get_aliases(context, data_dict):


def _get_resources(context, alias):
''' Get a list of resources for an alias. There could be more than one alias
in a resource_dict. '''
'''Get a list of resources for an alias. There could be more than one alias
in a resource_dict.'''
alias_sql = sqlalchemy.text(
u'''SELECT alias_of FROM "_table_metadata"
WHERE name = :alias AND alias_of IS NOT NULL''')
Expand Down Expand Up @@ -708,7 +707,7 @@ def _to_full_text(fields, record):


def _where(field_ids, data_dict):
'Return a SQL WHERE clause from data_dict filters and q'
'''Return a SQL WHERE clause from data_dict filters and q'''
filters = data_dict.get('filters', {})

if not isinstance(filters, dict):
Expand Down Expand Up @@ -794,9 +793,8 @@ def _sort(context, data_dict, field_ids):


def _insert_links(data_dict, limit, offset):
''' Adds link to the next/prev part (same limit, offset=offset+limit)
and the resource page.
'''
'''Adds link to the next/prev part (same limit, offset=offset+limit)
and the resource page.'''
data_dict['_links'] = {}

# get the url from the request
Expand Down
47 changes: 21 additions & 26 deletions ckanext/datastore/plugin.py
View file
Expand Up @@ -39,29 +39,29 @@ def configure(self, config):
# that we should ignore the following tests.
import sys
if sys.argv[0].split('/')[-1] == 'paster' and 'datastore' in sys.argv[1:]:
log.warn("Omitting permission checks because you are "
"running paster commands.")
log.warn('Omitting permission checks because you are '
'running paster commands.')
return

self.ckan_url = self.config['sqlalchemy.url']
self.write_url = self.config['ckan.datastore.write_url']
if self.legacy_mode:
self.read_url = self.write_url
log.warn("Legacy mode active. "
"The sql search will not be available.")
log.warn('Legacy mode active. '
'The sql search will not be available.')
else:
self.read_url = self.config['ckan.datastore.read_url']

if not model.engine_is_pg():
log.warn("We detected that you do not use a PostgreSQL "
"database. The DataStore will NOT work and DataStore "
"tests will be skipped.")
log.warn('We detected that you do not use a PostgreSQL '
'database. The DataStore will NOT work and DataStore '
'tests will be skipped.')
return

if self._is_read_only_database():
log.warn("We detected that CKAN is running on a read "
"only database. Permission checks and the creation "
"of _table_metadata are skipped.")
log.warn('We detected that CKAN is running on a read '
'only database. Permission checks and the creation '
'of _table_metadata are skipped.')
else:
self._check_urls_and_permissions()

Expand Down Expand Up @@ -111,25 +111,23 @@ def _check_urls_and_permissions(self):
# so that no harmful queries can be made

if self._same_ckan_and_datastore_db():
self._log_or_raise("CKAN and DataStore database "
"cannot be the same.")
self._log_or_raise('CKAN and DataStore database '
'cannot be the same.')

# in legacy mode, the read and write url are ths same (both write url)
# consequently the same url check and and write privilege check
# don't make sense
if not self.legacy_mode:
if self._same_read_and_write_url():
self._log_or_raise("The write and read-only database "
"connection urls are the same.")
self._log_or_raise('The write and read-only database '
'connection urls are the same.')

if not self._read_connection_has_correct_privileges():
self._log_or_raise("The read-only user has write privileges.")
self._log_or_raise('The read-only user has write privileges.')

def _is_read_only_database(self):
'''
Returns True if no connection has CREATE privileges on the public
schema. This is the case if replication is enabled.
'''
''' Returns True if no connection has CREATE privileges on the public
schema. This is the case if replication is enabled.'''
for url in [self.ckan_url, self.write_url, self.read_url]:
connection = db._get_engine(None,
{'connection_url': url}).connect()
Expand All @@ -140,9 +138,7 @@ def _is_read_only_database(self):
return True

def _same_ckan_and_datastore_db(self):
'''
Returns True if the CKAN and DataStore db are the same
'''
'''Returns True if the CKAN and DataStore db are the same'''
return self._get_db_from_url(self.ckan_url) == self._get_db_from_url(self.read_url)

def _get_db_from_url(self, url):
Expand All @@ -152,21 +148,20 @@ def _same_read_and_write_url(self):
return self.write_url == self.read_url

def _read_connection_has_correct_privileges(self):
'''
Returns True if the right permissions are set for the read only user.
''' Returns True if the right permissions are set for the read only user.
A table is created by the write user to test the read only user.
'''
write_connection = db._get_engine(None,
{'connection_url': self.write_url}).connect()
read_connection = db._get_engine(None,
{'connection_url': self.read_url}).connect()

drop_foo_sql = u"DROP TABLE IF EXISTS _foo"
drop_foo_sql = u'DROP TABLE IF EXISTS _foo'

write_connection.execute(drop_foo_sql)

try:
write_connection.execute(u"CREATE TABLE _foo ()")
write_connection.execute(u'CREATE TABLE _foo ()')
for privilege in ['INSERT', 'UPDATE', 'DELETE']:
test_privilege_sql = u"SELECT has_table_privilege('_foo', '{privilege}')"
sql = test_privilege_sql.format(privilege=privilege)
Expand Down

0 comments on commit 705c39e

Please sign in to comment.