Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel or delete stale pending and failed exports #1086

Merged
merged 5 commits into from
Aug 11, 2017
Merged

Cancel or delete stale pending and failed exports #1086

merged 5 commits into from
Aug 11, 2017

Conversation

moshthepitt
Copy link
Contributor

Fixes: #1068

Create periodic tasks which:

  • Find old pending exports and mark them as failed.

  • Find old failed exports and delete them

The 'oldness' is determined by a configurable setting.

Fixes: #1068

Create periodic tasks which:

- Find old pending exports and mark them as failed.

- Find old failed exports and delete them

The 'oldness' is determined by a configurable setting.
Delete old failed exports
"""
h = settings.EXPORT_TASK_LIFESPAN
time_threshold = datetime.now() - timedelta(hours=h)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably use the time zone aware django.utils.timezone.now().
The h could probably be more descriptive like task_lifespan.

exports = Export.objects.filter(internal_status=Export.FAILED,
created_on__lt=time_threshold)
for export in exports:
delete_export.delay(export.id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you chose to delete the export in a different task considering this is already task? Possible use queryset deletion, exports.delet().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had seen that there was a task created to specifically delete exports that was used elsewhere, and so I chose to use it - to keep things consistent.


def test_check_pending_exports(self):
result = check_pending_exports.delay()
self.assertTrue(result)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably check that the function deletes exports and not that a task is created.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. And implemented.

Improve tests to not only test that the celery tasks below are working:

- onadata.apps.viewer.tasks.check_pending_exports
- onadata.apps.viewer.tasks.delete_old_failed_exports

But also test that they have the desired effect on pending and failed exports,
respectively.
@ukanga ukanga changed the title Deal with old exports Cancel or delete stale pending and/or failed exports Aug 11, 2017
@ukanga ukanga changed the title Cancel or delete stale pending and/or failed exports Cancel or delete stale pending and failed exports Aug 11, 2017
@ukanga ukanga merged commit c1c7fca into master Aug 11, 2017
@ukanga ukanga deleted the issue-1068 branch August 11, 2017 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants