Merge pull request conwetlab#51 from qld-gov-au/QOL-7970-CKAN-2.9

[QOL-7970] clean up for Flake8 and Python 3

.ahoy.yml

@@ -127,7 +127,7 @@ commands:
       ahoy cli "behave ${*:-/app/test/features}" || \
       [ "${ALLOW_BDD_FAIL:-0}" -eq 1 ]
       ahoy stop-mailmock
-      ahoy stop-ckan-job-worker     
+      ahoy stop-ckan-job-worker
 
 
   start-mailmock:
@@ -147,8 +147,9 @@ commands:
     cmd: |
       ahoy title 'Starting CKAN background job worker'
       ahoy cli "cd /usr/lib/ckan/default/src/ckan && \
-                paster jobs clear --config=/app/ckan/default/production.ini && \
-                paster jobs worker default --config=/app/ckan/default/production.ini"
+                export CKAN_INI=/app/ckan/default/production.ini &&
+                /app/scripts/ckan_cli jobs clear && \
+                /app/scripts/ckan_cli jobs worker default"
 
   stop-ckan-job-worker:
     usage: Stops CKAN background job worker

.docker/Dockerfile.ckan

@@ -30,7 +30,8 @@ RUN fix-permissions /app/ckan \
     && chmod +x /app/scripts/create-test-data.sh \
     && chmod +x /app/scripts/init.sh \
     && chmod +x /app/scripts/init-ext.sh \
-    && chmod +x /app/scripts/serve.sh
+    && chmod +x /app/scripts/serve.sh \
+    && chmod +x /app/scripts/ckan_cli
 
 # Add current extension and files.
 COPY ckanext /app/ckanext

.docker/scripts/ckan_cli

@@ -0,0 +1,68 @@
+#!/bin/sh
+
+# Call either 'ckan' (from CKAN >= 2.9) or 'paster' (from CKAN <= 2.8)
+# with appropriate syntax, depending on what is present on the system.
+# This is intended to smooth the upgrade process from 2.8 to 2.9.
+# Eg:
+#   ckan_cli jobs list
+# could become either:
+#   paster --plugin=ckan jobs list -c /etc/ckan/default/production.ini
+# or:
+#   ckan -c /etc/ckan/default/production.ini jobs list
+
+# This script is aware of the VIRTUAL_ENV environment variable, and will
+# attempt to respect it with similar behaviour to commands like 'pip'.
+# Eg placing this script in a virtualenv 'bin' directory will cause it
+# to call the 'ckan' or 'paster' command in that directory, while
+# placing this script elsewhere will cause it to rely on the VIRTUAL_ENV
+# variable, or if that is not set, the system PATH.
+
+# Since the positioning of the CKAN configuration file is central to the
+# differences between 'paster' and 'ckan', this script needs to be aware
+# of the config file location. It will use the CKAN_INI environment
+# variable if it exists, or default to /etc/ckan/default/production.ini.
+
+# If 'paster' is being used, the default plugin is 'ckan'. A different
+# plugin can be specified by setting the PASTER_PLUGIN environment
+# variable. This variable is irrelevant if using the 'ckan' command.
+
+CKAN_INI="${CKAN_INI:-/etc/ckan/default/production.ini}"
+PASTER_PLUGIN="${PASTER_PLUGIN:-ckan}"
+# First, look for a command alongside this file
+ENV_DIR=$(dirname "$0")
+if [ -f "$ENV_DIR/ckan" ]; then
+    COMMAND=ckan
+elif [ -f "$ENV_DIR/paster" ]; then
+    COMMAND=paster
+elif [ "$VIRTUAL_ENV" != "" ]; then
+    # If command not found alongside this file, check the virtualenv
+    ENV_DIR="$VIRTUAL_ENV/bin"
+    if [ -f "$ENV_DIR/ckan" ]; then
+        COMMAND=ckan
+    elif [ -f "$ENV_DIR/paster" ]; then
+        COMMAND=paster
+    fi
+else
+    # if no virtualenv is active, try the system path
+    if (which ckan > /dev/null 2>&1); then
+        ENV_DIR=$(dirname $(which ckan))
+        COMMAND=ckan
+    elif (which paster > /dev/null 2>&1); then
+        ENV_DIR=$(dirname $(which paster))
+        COMMAND=paster
+    else
+        echo "Unable to locate 'ckan' or 'paster' command" >&2
+        exit 1
+    fi
+fi
+
+if [ "$COMMAND" = "ckan" ]; then
+    echo "Using 'ckan' command from $ENV_DIR..." >&2
+    exec $ENV_DIR/ckan -c ${CKAN_INI} "$@"
+elif [ "$COMMAND" = "paster" ]; then
+    echo "Using 'paster' command from $ENV_DIR..." >&2
+    exec $ENV_DIR/paster --plugin=$PASTER_PLUGIN "$@" -c ${CKAN_INI}
+else
+    echo "Unable to locate 'ckan' or 'paster' command in $ENV_DIR" >&2
+    exit 1
+fi

.docker/scripts/create-test-data.sh

@@ -6,17 +6,16 @@
 set -e
 
 CKAN_ACTION_URL=http://ckan:3000/api/action
-CKAN_INI_FILE=/app/ckan/default/production.ini
+export CKAN_INI=/app/ckan/default/production.ini
 
-. /app/ckan/default/bin/activate \
-    && cd /app/ckan/default/src/ckan
+. /app/ckan/default/bin/activate
 
 # We know the "admin" sysadmin account exists, so we'll use her API KEY to create further data
-API_KEY=$(paster --plugin=ckan user admin -c ${CKAN_INI_FILE} | tr -d '\n' | sed -r 's/^(.*)apikey=(\S*)(.*)/\2/')
+API_KEY=$(/app/scripts/ckan_cli user admin | tr -d '\n' | sed -r 's/^(.*)apikey=(\S*)(.*)/\2/')
 
-# # 
+# #
 ##
-# BEGIN: Add sysadmin config values. 
+# BEGIN: Add sysadmin config values.
 # This needs to be done before closing datarequests as they require the below config values
 #
 echo "Adding ckan.datarequests.closing_circumstances:"
@@ -37,10 +36,10 @@ TEST_ORG_TITLE="Test"
 
 echo "Creating test users for ${TEST_ORG_TITLE} Organisation:"
 
-paster --plugin=ckan user add ckan_user email=ckan_user@localhost password=password -c ${CKAN_INI_FILE}
-paster --plugin=ckan user add test_org_admin email=test_org_admin@localhost password=password -c ${CKAN_INI_FILE}
-paster --plugin=ckan user add test_org_editor email=test_org_editor@localhost password=password -c ${CKAN_INI_FILE}
-paster --plugin=ckan user add test_org_member email=test_org_member@localhost password=password -c ${CKAN_INI_FILE}
+/app/scripts/ckan_cli user add ckan_user email=ckan_user@localhost password=password
+/app/scripts/ckan_cli user add test_org_admin email=test_org_admin@localhost password=password
+/app/scripts/ckan_cli user add test_org_editor email=test_org_editor@localhost password=password
+/app/scripts/ckan_cli user add test_org_member email=test_org_member@localhost password=password
 
 echo "Creating ${TEST_ORG_TITLE} Organisation:"
 
@@ -78,9 +77,9 @@ DR_ORG_TITLE="Open Data Administration (data requests)"
 
 echo "Creating test users for ${DR_ORG_TITLE} Organisation:"
 
-paster --plugin=ckan user add dr_admin email=dr_admin@localhost password=password -c ${CKAN_INI_FILE}
-paster --plugin=ckan user add dr_editor email=dr_editor@localhost password=password -c ${CKAN_INI_FILE}
-paster --plugin=ckan user add dr_member email=dr_member@localhost password=password -c ${CKAN_INI_FILE}
+/app/scripts/ckan_cli user add dr_admin email=dr_admin@localhost password=password
+/app/scripts/ckan_cli user add dr_editor email=dr_editor@localhost password=password
+/app/scripts/ckan_cli user add dr_member email=dr_member@localhost password=password
 
 echo "Creating ${DR_ORG_TITLE} Organisation:"
 
@@ -138,8 +137,8 @@ curl -L -s --header "Authorization: ${API_KEY}" \
 # END.
 #
 
-# Use CKAN's built-in paster command for creating some test datasets...
-paster create-test-data -c ${CKAN_INI_FILE}
+# Use CKAN's built-in commands for creating some test datasets...
+/app/scripts/ckan_cli create-test-data
 
 # Datasets need to be assigned to an organisation
 

.docker/scripts/init-ext.sh

@@ -9,8 +9,9 @@ set -e
 pip install -r "/app/requirements.txt"
 pip install -r "/app/requirements-dev.txt"
 python setup.py develop
+installed_name=$(grep '^\s*name=' setup.py |sed "s|[^']*'\([-a-zA-Z0-9]*\)'.*|\1|")
 
 # Validate that the extension was installed correctly.
-if ! pip list | grep ckanext-datarequests > /dev/null; then echo "Unable to find the extension in the list"; exit 1; fi
+if ! pip list | grep "$installed_name" > /dev/null; then echo "Unable to find the extension in the list"; exit 1; fi
 
 deactivate

.docker/scripts/init.sh

@@ -7,25 +7,25 @@ set -e
 CKAN_USER_NAME="${CKAN_USER_NAME:-admin}"
 CKAN_USER_PASSWORD="${CKAN_USER_PASSWORD:-password}"
 CKAN_USER_EMAIL="${CKAN_USER_EMAIL:-admin@localhost}"
+export CKAN_INI=/app/ckan/default/production.ini
 
-. /app/ckan/default/bin/activate \
-  && cd /app/ckan/default/src/ckan \
-  && paster db clean -c /app/ckan/default/production.ini \
-  && paster db init -c /app/ckan/default/production.ini \
-  && paster --plugin=ckan user add "${CKAN_USER_NAME}" email="${CKAN_USER_EMAIL}" password="${CKAN_USER_PASSWORD}" -c /app/ckan/default/production.ini \
-  && paster --plugin=ckan sysadmin add "${CKAN_USER_NAME}" -c /app/ckan/default/production.ini
+. /app/ckan/default/bin/activate
+/app/scripts/ckan_cli db clean
+/app/scripts/ckan_cli db init
+/app/scripts/ckan_cli user add "${CKAN_USER_NAME}" email="${CKAN_USER_EMAIL}" password="${CKAN_USER_PASSWORD}"
+/app/scripts/ckan_cli sysadmin add "${CKAN_USER_NAME}"
 
 # Initialise the Comments database tables
-paster --plugin=ckanext-ytp-comments initdb --config=/app/ckan/default/production.ini
+PASTER_PLUGIN=ckanext-ytp-comments /app/scripts/ckan_cli initdb
 
 # Initialise the archiver database tables
-paster --plugin=ckanext-archiver archiver init --config=/app/ckan/default/production.ini
+PASTER_PLUGIN=ckanext-archiver /app/scripts/ckan_cli archiver init
 
 # Initialise the reporting database tables
-paster --plugin=ckanext-report report initdb --config=/app/ckan/default/production.ini
+PASTER_PLUGIN=ckanext-report /app/scripts/ckan_cli report initdb
 
 # Initialise the QA database tables
-paster --plugin=ckanext-qa qa init --config=/app/ckan/default/production.ini
+PASTER_PLUGIN=ckanext-qa /app/scripts/ckan_cli qa init
 
 # Create some base test data
 . /app/scripts/create-test-data.sh

.flake8

@@ -15,5 +15,7 @@ max-complexity = 10
 
 # List ignore rules one per line.
 ignore =
+    E241
+    E266
     E501
     C901

ckanext/datarequests/actions.py

@@ -18,16 +18,14 @@
 # along with CKAN Data Requests Extension. If not, see <http://www.gnu.org/licenses/>.
 
 
-import ckan.lib.base as base
-import ckan.model as model
-import ckan.plugins as plugins
 import constants
 import datetime
 import cgi
 import db
 import logging
 import validator
-import ckan.lib.mailer as mailer
+from ckan import model, plugins
+from ckan.lib import base, mailer
 
 from pylons import config
 
@@ -201,7 +199,7 @@ def create_datarequest(context, data_dict):
     :type organization_id: string
 
     :returns: A dict with the data request (id, user_id, title, description,
-        organization_id, open_time, accepted_dataset, close_time, closed, 
+        organization_id, open_time, accepted_dataset, close_time, closed,
         followers)
     :rtype: dict
     '''
@@ -230,7 +228,7 @@ def create_datarequest(context, data_dict):
     datarequest_dict = _dictize_datarequest(data_req)
 
     if datarequest_dict['organization']:
-        users = set([user['id'] for user in datarequest_dict['organization']['users']])
+        users = {user['id'] for user in datarequest_dict['organization']['users']}
         users.discard(context['auth_user_obj'].id)
         _send_mail(users, 'new_datarequest', datarequest_dict)
 
@@ -250,7 +248,7 @@ def show_datarequest(context, data_dict):
     :type id: string
 
     :returns: A dict with the data request (id, user_id, title, description,
-        organization_id, open_time, accepted_dataset, close_time, closed, 
+        organization_id, open_time, accepted_dataset, close_time, closed,
         followers)
     :rtype: dict
     '''
@@ -302,7 +300,7 @@ def update_datarequest(context, data_dict):
     :type organization_id: string
 
     :returns: A dict with the data request (id, user_id, title, description,
-        organization_id, open_time, accepted_dataset, close_time, closed, 
+        organization_id, open_time, accepted_dataset, close_time, closed,
         followers)
     :rtype: dict
     '''
@@ -455,7 +453,7 @@ def list_datarequests(context, data_dict):
                 'display_name': organization.get('display_name'),
                 'count': no_processed_organization_facet[organization_id]
             })
-        except:
+        except Exception:
             pass
 
     state_facet = []
@@ -493,7 +491,7 @@ def delete_datarequest(context, data_dict):
     :type id: string
 
     :returns: A dict with the data request (id, user_id, title, description,
-        organization_id, open_time, accepted_dataset, close_time, closed, 
+        organization_id, open_time, accepted_dataset, close_time, closed,
         followers)
     :rtype: dict
     '''
@@ -538,7 +536,7 @@ def close_datarequest(context, data_dict):
     :type accepted_dataset_id: string
 
     :returns: A dict with the data request (id, user_id, title, description,
-        organization_id, open_time, accepted_dataset, close_time, closed, 
+        organization_id, open_time, accepted_dataset, close_time, closed,
         followers)
     :rtype: dict
 
@@ -821,11 +819,11 @@ def delete_datarequest_comment(context, data_dict):
 
 def follow_datarequest(context, data_dict):
     '''
-    Action to follow a data request. Access rights will be cheked before 
+    Action to follow a data request. Access rights will be cheked before
     following a datarequest and a NotAuthorized exception will be risen if the
     user is not allowed to follow the given datarequest. ValidationError will
     be risen if the datarequest ID is not included or if the user is already
-    following the datarequest. ObjectNotFound will be risen if the given 
+    following the datarequest. ObjectNotFound will be risen if the given
     datarequest does not exist.
 
     :param id: The ID of the datarequest to be followed
@@ -873,11 +871,11 @@ def follow_datarequest(context, data_dict):
 
 def unfollow_datarequest(context, data_dict):
     '''
-    Action to unfollow a data request. Access rights will be cheked before 
+    Action to unfollow a data request. Access rights will be cheked before
     unfollowing a datarequest and a NotAuthorized exception will be risen if
     the user is not allowed to unfollow the given datarequest. ValidationError
-    will be risen if the datarequest ID is not included in the request. 
-    ObjectNotFound will be risen if the user is not following the given 
+    will be risen if the datarequest ID is not included in the request.
+    ObjectNotFound will be risen if the user is not following the given
     datarequest.
 
     :param id: The ID of the datarequest to be unfollowed

ckanext/datarequests/controllers/ui_controller.py

@@ -19,16 +19,16 @@
 
 import logging
 
-import ckan.lib.base as base
-import ckan.model as model
-import ckan.plugins as plugins
-import ckan.lib.helpers as helpers
-import ckanext.datarequests.constants as constants
 import functools
 import re
+import six
 
+from six.moves.urllib.parse import urlencode
+
+from ckan import model, plugins
 from ckan.common import request
-from urllib import urlencode
+from ckan.lib import base, helpers
+from ckanext.datarequests import constants
 
 
 _link = re.compile(r'(?:(https?://)|(www\.))(\S+\b/?)([!"#$%&\'()*+,\-./:;<=>?@[\\\]^_`{|}~]*)(\s|$)', re.I)
@@ -48,7 +48,7 @@ def _get_errors_summary(errors):
 
 
 def _encode_params(params):
-    return [(k, v.encode('utf-8') if isinstance(v, basestring) else str(v))
+    return [(k, v.encode('utf-8') if isinstance(v, six.string_types) else str(v))
             for k, v in params]
 
 
@@ -84,7 +84,7 @@ def _get_context(self):
     def _show_index(self, user_id, organization_id, include_organization_facet, url_func, file_to_render):
 
         def pager_url(state=None, sort=None, q=None, page=None):
-            params = list()
+            params = []
 
             if q:
                 params.append(('q', q))
@@ -220,7 +220,7 @@ def show(self, id):
             context_ignore_auth['ignore_auth'] = True
 
             return tk.render('datarequests/show.html')
-        except tk.ObjectNotFound as e:
+        except tk.ObjectNotFound:
             tk.abort(404, tk._('Data Request %s not found') % id)
         except tk.NotAuthorized as e:
             log.warn(e)
@@ -286,9 +286,11 @@ def close(self, id):
         # Basic intialization
         c.datarequest = {}
 
-        def _return_page(errors={}, errors_summary={}):
-            # Get datasets (if the data req belongs to an organization, only the one that
-            # belongs to the organization are shown)
+        def _return_page(errors=None, errors_summary=None):
+            errors = errors or {}
+            errors_summary = errors_summary or {}
+            # Get datasets (if the data req belongs to an organization,
+            # only the ones that belong to the organization are shown)
             organization_id = c.datarequest.get('organization_id', '')
             if organization_id:
                 base_datasets = tk.get_action('organization_show')({'ignore_auth': True}, {'id': organization_id, 'include_datasets': True})['packages']