diff --git a/onadata/apps/viewer/models/export.py b/onadata/apps/viewer/models/export.py index 2a6dd35174..4088a21ee0 100644 --- a/onadata/apps/viewer/models/export.py +++ b/onadata/apps/viewer/models/export.py @@ -1,5 +1,8 @@ +# -*- coding=utf-8 -*- +""" +Export model. +""" import os -import hashlib from tempfile import NamedTemporaryFile from django.core.files.storage import get_storage_class @@ -15,17 +18,17 @@ EXPORT_QUERY_KEY = 'query' +# pylint: disable=unused-argument def export_delete_callback(sender, **kwargs): + """ + Delete export file when an export object is deleted. + """ export = kwargs['instance'] storage = get_storage_class()() if export.filepath and storage.exists(export.filepath): storage.delete(export.filepath) -def md5hash(string): - return hashlib.md5(string).hexdigest() - - def get_export_options_query_kwargs(options): """ Get dict with options JSONField lookups for export options field @@ -47,6 +50,9 @@ class Export(models.Model): """ class ExportTypeError(Exception): + """ + ExportTypeError exception class. + """ def __unicode__(self): return _(u"Invalid export type specified") @@ -141,11 +147,13 @@ class Meta: app_label = "viewer" unique_together = (("xform", "filename"),) - def save(self, *args, **kwargs): + def __unicode__(self): + return u'%s - %s (%s)' % (self.export_type, self.xform, self.filename) + + def save(self, *args, **kwargs): # pylint: disable=arguments-differ if not self.pk and self.xform: # if new, check if we've hit our limit for exports for this form, # if so, delete oldest - # TODO: let user know that last export will be deleted num_existing_exports = Export.objects.filter( xform=self.xform, export_type=self.export_type).count() @@ -168,49 +176,70 @@ def _delete_oldest_export(cls, xform, export_type): @property def is_pending(self): + """ + Return True if an export status is pending. + """ return self.status == Export.PENDING @property def is_successful(self): + """ + Return True if an export status successful. + """ return self.status == Export.SUCCESSFUL @property def status(self): + """ + Return the status [FAILED|PENDING|SUCCESSFUL] of an export. + """ if self.filename: # need to have this since existing models will have their # internal_status set to PENDING - the default return Export.SUCCESSFUL elif self.internal_status == Export.FAILED: return Export.FAILED - else: - return Export.PENDING + + return Export.PENDING def set_filename(self, filename): + """ + Set the filename of an export and mark internal_status as + Export.SUCCESSFUL. + """ self.filename = filename self.internal_status = Export.SUCCESSFUL self._update_filedir() def _update_filedir(self): - assert(self.filename) + assert self.filename self.filedir = os.path.join(self.xform.user.username, 'exports', self.xform.id_string, self.export_type) @property def filepath(self): + """ + Return the file path of an export file, None if the file does not + exist. + """ if self.filedir and self.filename: return os.path.join(self.filedir, self.filename) return None @property def full_filepath(self): + """ + Return the full filepath of an export file, None if the file does not + exist. + """ if self.filepath: default_storage = get_storage_class()() try: return default_storage.path(self.filepath) except NotImplementedError: # read file from s3 - name, ext = os.path.splitext(self.filepath) + _name, ext = os.path.splitext(self.filepath) tmp = NamedTemporaryFile(suffix=ext, delete=False) f = default_storage.open(self.filepath) tmp.write(f.read()) @@ -219,7 +248,13 @@ def full_filepath(self): return None @classmethod - def exports_outdated(cls, xform, export_type, options={}): + def exports_outdated(cls, xform, export_type, options=None): + """ + Return True if export is outdated or there is no export matching the + export_type with the specified options. + """ + if options is None: + options = {} # get newest export for xform try: export_options = get_export_options_query_kwargs(options) @@ -234,13 +269,16 @@ def exports_outdated(cls, xform, export_type, options={}): and xform.time_of_last_submission_update() is not None: return latest_export.time_of_last_submission <\ xform.time_of_last_submission_update() - else: - # return true if we can't determine the status, to force - # auto-generation - return True + + # return true if we can't determine the status, to force + # auto-generation + return True @classmethod def is_filename_unique(cls, xform, filename): + """ + Return True if the filename is unique. + """ return Export.objects.filter( xform=xform, filename=filename).count() == 0 diff --git a/onadata/apps/viewer/tasks.py b/onadata/apps/viewer/tasks.py index 65c6fc5f58..0c703b0e01 100644 --- a/onadata/apps/viewer/tasks.py +++ b/onadata/apps/viewer/tasks.py @@ -1,3 +1,7 @@ +# -*- coding=utf-8 -*- +""" +Export tasks. +""" import sys from datetime import timedelta @@ -17,15 +21,15 @@ EXPORT_QUERY_KEY = 'query' -def _get_export_object(id): +def _get_export_object(export_id): try: - return Export.objects.get(id=id) + return Export.objects.get(id=export_id) except Export.DoesNotExist: - if len(getattr(settings, 'SLAVE_DATABASES', [])): + if getattr(settings, 'SLAVE_DATABASES', []): from multidb.pinning import use_master with use_master: - return Export.objects.get(id=id) + return Export.objects.get(id=export_id) raise Export.DoesNotExist @@ -40,6 +44,9 @@ def _get_export_details(username, id_string, export_id): def create_async_export(xform, export_type, query, force_xlsx, options=None): + """ + Starts asynchronous export tasks and returns an export object. + """ username = xform.user.username id_string = xform.id_string @@ -100,13 +107,16 @@ def _create_export(xform, export_type, options): @task(track_started=True) def create_xls_export(username, id_string, export_id, **options): + """ + XLS export task. + """ # we re-query the db instead of passing model objects according to # http://docs.celeryproject.org/en/latest/userguide/tasks.html#state force_xlsx = options.get("force_xlsx", True) options["extension"] = 'xlsx' if force_xlsx else 'xls' try: - export = _get_export_object(id=export_id) + export = _get_export_object(export_id) except Export.DoesNotExist: # no export for this ID return None. return None @@ -122,9 +132,10 @@ def create_xls_export(username, id_string, export_id, **options): # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("XLS Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "XLS Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) # Raise for now to let celery know we failed # - doesnt seem to break celery` raise @@ -134,9 +145,12 @@ def create_xls_export(username, id_string, export_id, **options): @task(track_started=True) def create_csv_export(username, id_string, export_id, **options): + """ + CSV export task. + """ # we re-query the db instead of passing model objects according to # http://docs.celeryproject.org/en/latest/userguide/tasks.html#state - export = _get_export_object(id=export_id) + export = _get_export_object(export_id) try: # though export is not available when for has 0 submissions, we @@ -154,9 +168,10 @@ def create_csv_export(username, id_string, export_id, **options): # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("CSV Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "CSV Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) raise else: return gen_export.id @@ -164,10 +179,13 @@ def create_csv_export(username, id_string, export_id, **options): @task(track_started=True) def create_kml_export(username, id_string, export_id, **options): + """ + KML export task. + """ # we re-query the db instead of passing model objects according to # http://docs.celeryproject.org/en/latest/userguide/tasks.html#state - export = _get_export_object(id=export_id) + export = _get_export_object(export_id) try: # though export is not available when for has 0 submissions, we # catch this since it potentially stops celery @@ -183,9 +201,10 @@ def create_kml_export(username, id_string, export_id, **options): export.save() # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("KML Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "KML Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) raise else: return gen_export.id @@ -193,10 +212,13 @@ def create_kml_export(username, id_string, export_id, **options): @task(track_started=True) def create_osm_export(username, id_string, export_id, **options): + """ + OSM export task. + """ # we re-query the db instead of passing model objects according to # http://docs.celeryproject.org/en/latest/userguide/tasks.html#state - export = _get_export_object(id=export_id) + export = _get_export_object(export_id) try: # though export is not available when for has 0 submissions, we # catch this since it potentially stops celery @@ -212,9 +234,10 @@ def create_osm_export(username, id_string, export_id, **options): export.save() # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("OSM Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "OSM Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) raise else: return gen_export.id @@ -222,7 +245,10 @@ def create_osm_export(username, id_string, export_id, **options): @task(track_started=True) def create_zip_export(username, id_string, export_id, **options): - export = _get_export_object(id=export_id) + """ + Attachments zip export task. + """ + export = _get_export_object(export_id) try: gen_export = generate_attachments_zip_export( Export.ZIP_EXPORT, @@ -236,9 +262,9 @@ def create_zip_export(username, id_string, export_id, **options): export.save() # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("Zip Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e) + report_exception( + "Zip Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e) raise else: if not settings.TESTING_MODE: @@ -250,7 +276,10 @@ def create_zip_export(username, id_string, export_id, **options): @task(track_started=True) def create_csv_zip_export(username, id_string, export_id, **options): - export = _get_export_object(id=export_id) + """ + CSV zip export task. + """ + export = _get_export_object(export_id) options["extension"] = Export.ZIP_EXPORT try: # though export is not available when for has 0 submissions, we @@ -262,9 +291,10 @@ def create_csv_zip_export(username, id_string, export_id, **options): export.save() # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("CSV ZIP Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "CSV ZIP Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) raise else: return gen_export.id @@ -272,7 +302,10 @@ def create_csv_zip_export(username, id_string, export_id, **options): @task(track_started=True) def create_sav_zip_export(username, id_string, export_id, **options): - export = _get_export_object(id=export_id) + """ + SPSS sav export task. + """ + export = _get_export_object(export_id) options["extension"] = Export.ZIP_EXPORT try: # though export is not available when for has 0 submissions, we @@ -284,9 +317,10 @@ def create_sav_zip_export(username, id_string, export_id, **options): export.save() # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("SAV ZIP Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "SAV ZIP Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) raise else: return gen_export.id @@ -294,6 +328,9 @@ def create_sav_zip_export(username, id_string, export_id, **options): @task(track_started=True) def create_external_export(username, id_string, export_id, **options): + """ + XLSReport export task. + """ export = get_object_or_404(Export, id=export_id) try: @@ -311,9 +348,10 @@ def create_external_export(username, id_string, export_id, **options): export.save() # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("External Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "External Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) raise else: return gen_export.id @@ -321,10 +359,13 @@ def create_external_export(username, id_string, export_id, **options): @task(track_started=True) def create_google_sheet_export(username, id_string, export_id, **options): + """ + Google Sheets export task. + """ # we re-query the db instead of passing model objects according to # http://docs.celeryproject.org/en/latest/userguide/tasks.html#state try: - export = _get_export_object(id=export_id) + export = _get_export_object(export_id) # though export is not available when for has 0 submissions, we # catch this since it potentially stops celery gen_export = generate_export(Export.GOOGLE_SHEETS_EXPORT, export.xform, @@ -334,9 +375,10 @@ def create_google_sheet_export(username, id_string, export_id, **options): export.save() # mail admins details = _get_export_details(username, id_string, export_id) - report_exception("Google Export Exception: Export ID - " - "%(export_id)s, /%(username)s/%(id_string)s" % - details, e, sys.exc_info()) + report_exception( + "Google Export Exception: Export ID - " + "%(export_id)s, /%(username)s/%(id_string)s" % details, e, + sys.exc_info()) raise else: return gen_export.id @@ -344,8 +386,11 @@ def create_google_sheet_export(username, id_string, export_id, **options): @task(track_started=True) def delete_export(export_id): + """ + Delete export task with id export_id. + """ try: - export = _get_export_object(id=export_id) + export = _get_export_object(export_id) except Export.DoesNotExist: pass else: @@ -355,15 +400,15 @@ def delete_export(export_id): @task(ignore_result=True) -def mark_expired_pending_exports_as_failed(): +def mark_expired_pending_exports_as_failed(): # pylint: disable=invalid-name """ Exports that have not completed within a set time should be marked as failed """ task_lifespan = settings.EXPORT_TASK_LIFESPAN time_threshold = timezone.now() - timedelta(hours=task_lifespan) - exports = Export.objects.filter(internal_status=Export.PENDING, - created_on__lt=time_threshold) + exports = Export.objects.filter( + internal_status=Export.PENDING, created_on__lt=time_threshold) exports.update(internal_status=Export.FAILED) @@ -374,6 +419,6 @@ def delete_expired_failed_exports(): """ task_lifespan = settings.EXPORT_TASK_LIFESPAN time_threshold = timezone.now() - timedelta(hours=task_lifespan) - exports = Export.objects.filter(internal_status=Export.FAILED, - created_on__lt=time_threshold) + exports = Export.objects.filter( + internal_status=Export.FAILED, created_on__lt=time_threshold) exports.delete() diff --git a/onadata/libs/tests/utils/test_api_export_tools.py b/onadata/libs/tests/utils/test_api_export_tools.py index 28b52596b1..6d8420698f 100644 --- a/onadata/libs/tests/utils/test_api_export_tools.py +++ b/onadata/libs/tests/utils/test_api_export_tools.py @@ -1,3 +1,7 @@ +# -*- coding=utf-8 -*- +""" +Test api_export_tools module. +""" from collections import OrderedDict, defaultdict import mock @@ -17,6 +21,9 @@ class TestApiExportTools(TestBase): + """ + Test api_export_tools. + """ def _create_old_export(self, xform, export_type, options, filename=None): options = OrderedDict(sorted(options.items())) Export( @@ -25,10 +32,15 @@ def _create_old_export(self, xform, export_type, options, filename=None): options=options, filename=filename, internal_status=Export.SUCCESSFUL).save() + # pylint: disable=attribute-defined-outside-init self.export = Export.objects.filter( xform=xform, export_type=export_type)[0] + # pylint: disable=invalid-name def test_process_async_export_creates_new_export(self): + """ + Test process_async_export creates a new export. + """ self._publish_transportation_form_and_submit_instance() request = self.factory.post('/') request.user = self.user @@ -40,7 +52,11 @@ def test_process_async_export_creates_new_export(self): self.assertIn('job_uuid', resp) + # pylint: disable=invalid-name def test_process_async_export_returns_existing_export(self): + """ + Test process_async_export returns existing export. + """ settings.CELERY_ALWAYS_EAGER = True current_app.conf.CELERY_ALWAYS_EAGER = True @@ -64,9 +80,14 @@ def test_process_async_export_returns_existing_export(self): self.assertEquals(resp['job_status'], status_msg[SUCCESSFUL]) self.assertIn("export_url", resp) + # pylint: disable=invalid-name @mock.patch('onadata.libs.utils.api_export_tools.AsyncResult') def test_get_async_response_export_does_not_exist(self, AsyncResult): - class MockAsyncResult(object): + """ + Test get_async_response export does not exist. + """ + class MockAsyncResult(object): # pylint: disable=R0903 + """Mock AsyncResult""" def __init__(self): self.state = 'SUCCESS' self.result = 1 @@ -81,14 +102,20 @@ def __init__(self): with self.assertRaises(Http404): get_async_response('job_uuid', request, self.xform) + # pylint: disable=invalid-name @mock.patch('onadata.libs.utils.api_export_tools.AsyncResult') - def test_get_async_response_export_blacklog_limit(self, AsyncResult): - class MockAsyncResult(object): + def test_get_async_response_export_backlog_limit(self, AsyncResult): + """ + Test get_async_response export backlog limit exceeded. + """ + class MockAsyncResult(object): # pylint: disable=R0903 + """Mock AsyncResult""" def __init__(self): pass @property def state(self): + """Raise BacklogLimitExceeded""" raise BacklogLimitExceeded() AsyncResult.return_value = MockAsyncResult() @@ -102,6 +129,9 @@ def state(self): self.assertEqual(result, {'job_status': 'PENDING'}) def test_response_for_format(self): + """ + Test response format type. + """ self._publish_xlsx_file() xform = XForm.objects.filter().last() self.assertIsNotNone(xform)