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

Fix regression with publish task #4602

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions contentcuration/contentcuration/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2565,14 +2565,13 @@ def serialize_to_change_dict(self):
class CustomTaskMetadata(models.Model):
# Task_id for reference
task_id = models.CharField(
max_length=255, # Adjust the max_length as needed
max_length=255,
unique=True,
)
# user shouldn't be null, but in order to append the field, this needs to be allowed
user = models.ForeignKey(settings.AUTH_USER_MODEL, related_name="tasks", on_delete=models.CASCADE, null=True)
channel_id = DjangoUUIDField(db_index=True, null=True, blank=True)
progress = models.IntegerField(null=True, blank=True, validators=[MinValueValidator(0), MaxValueValidator(100)])
# a hash of the task name and kwargs for identifying repeat tasks
# A hash of the task name and kwargs for identifying repeat tasks
signature = models.CharField(null=True, blank=False, max_length=32)
date_created = models.DateTimeField(
auto_now_add=True,
Expand All @@ -2582,7 +2581,6 @@ class CustomTaskMetadata(models.Model):

class Meta:
indexes = [
# add index that matches query usage for signature
models.Index(
fields=['signature'],
name='task_result_signature',
Expand Down
44 changes: 44 additions & 0 deletions contentcuration/contentcuration/tests/test_exportchannel.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
from __future__ import absolute_import

import json
import os
import random
import string
import tempfile
import uuid

import pytest
from celery import states
from django.core.management import call_command
from django.db import connections
from django_celery_results.models import TaskResult
from kolibri_content import models as kolibri_models
from kolibri_content.router import cleanup_content_database_connection
from kolibri_content.router import get_active_content_database
Expand All @@ -29,6 +33,8 @@
from .testdata import slideshow
from .testdata import thumbnail_bytes
from contentcuration import models as cc
from contentcuration.models import CustomTaskMetadata
from contentcuration.utils.celery.tasks import generate_task_signature
from contentcuration.utils.publish import ChannelIncompleteError
from contentcuration.utils.publish import convert_channel_thumbnail
from contentcuration.utils.publish import create_content_database
Expand All @@ -37,6 +43,7 @@
from contentcuration.utils.publish import map_prerequisites
from contentcuration.utils.publish import MIN_SCHEMA_VERSION
from contentcuration.utils.publish import set_channel_icon_encoding
from contentcuration.viewsets.base import create_change_tracker

pytestmark = pytest.mark.django_db

Expand Down Expand Up @@ -553,3 +560,40 @@ def test_fill_published_fields(self):
self.assertTrue(channel.published_data)
self.assertIsNotNone(channel.published_data.get(0))
self.assertEqual(channel.published_data[0]['version_notes'], version_notes)


class PublishFailCleansUpTaskObjects(StudioTestCase):
def setUp(self):
super(PublishFailCleansUpTaskObjects, self).setUpBase()

def test_failed_task_objects_cleaned_up_when_publishing(self):
Copy link
Member

Choose a reason for hiding this comment

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

Nice regression test!

channel_id = self.channel.id
task_name = 'export-channel'
task_id = uuid.uuid4().hex
pk = 'ab684452f2ad4ba6a1426d6410139f60'
table = 'channel'
task_kwargs = json.dumps({'pk': pk, 'table': table})
signature = generate_task_signature(task_name, task_kwargs=task_kwargs, channel_id=channel_id)

TaskResult.objects.create(
task_id=task_id,
status=states.FAILURE,
task_name=task_name,
)

CustomTaskMetadata.objects.create(
task_id=task_id,
channel_id=channel_id,
user=self.user,
signature=signature
)

assert TaskResult.objects.filter(task_id=task_id).exists()
assert CustomTaskMetadata.objects.filter(task_id=task_id).exists()

with create_change_tracker(pk, table, channel_id, self.user, task_name):
assert not TaskResult.objects.filter(task_id=task_id).exists()
assert not CustomTaskMetadata.objects.filter(task_id=task_id).exists()
new_task_result = TaskResult.objects.filter(task_name=task_name, status=states.STARTED).first()
new_custom_task_metadata = CustomTaskMetadata.objects.get(channel_id=channel_id, user=self.user, signature=signature)
assert new_custom_task_metadata.task_id == new_task_result.task_id
9 changes: 6 additions & 3 deletions contentcuration/contentcuration/viewsets/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -936,9 +936,12 @@ def create_change_tracker(pk, table, channel_id, user, task_name):
# Clean up any previous tasks specific to this in case there were failures.
signature = generate_task_signature(task_name, task_kwargs=task_kwargs, channel_id=channel_id)

task_id_to_delete = CustomTaskMetadata.objects.filter(channel_id=channel_id, signature=signature)
if task_id_to_delete:
TaskResult.objects.filter(task_id=task_id_to_delete, task_name=task_name).delete()
custom_task_metadata_qs = CustomTaskMetadata.objects.filter(channel_id=channel_id, signature=signature)
if custom_task_metadata_qs.exists():
task_result_qs = TaskResult.objects.filter(task_id=custom_task_metadata_qs[0].task_id, task_name=task_name)
if task_result_qs.exists():
task_result_qs[0].delete()
custom_task_metadata_qs[0].delete()

task_id = uuid.uuid4().hex

Expand Down