Skip to content

Commit

Permalink
Update change tracker to re-raise; update copy operation to log excep…
Browse files Browse the repository at this point in the history
…tions
  • Loading branch information
bjester committed Jun 23, 2023
1 parent b90feb7 commit dfd6536
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 17 deletions.
1 change: 1 addition & 0 deletions contentcuration/contentcuration/viewsets/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,7 @@ def update_progress(progress=None):
task_object.status = states.FAILURE
task_object.traceback = traceback.format_exc()
task_object.save()
raise
finally:
if task_object.status == states.STARTED:
# No error reported, cleanup.
Expand Down
23 changes: 9 additions & 14 deletions contentcuration/contentcuration/viewsets/contentnode.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
from contentcuration.viewsets.sync.constants import CREATED
from contentcuration.viewsets.sync.constants import DELETED
from contentcuration.viewsets.sync.utils import generate_update_event

from contentcuration.viewsets.sync.utils import log_sync_exception

channel_query = Channel.objects.filter(main_tree__tree_id=OuterRef("tree_id"))

Expand Down Expand Up @@ -908,9 +908,11 @@ def copy_from_changes(self, changes):
for copy in changes:
# Copy change will have key, must also have other attributes, defined in `copy`
# Just pass as keyword arguments here to let copy do the validation
copy_errors = self.copy(copy["key"], **copy)
if copy_errors:
copy.update({"errors": copy_errors})
try:
self.copy(copy["key"], **copy)
except Exception as e:
log_sync_exception(e, user=self.request.user, change=copy)
copy["errors"] = [str(e)]
errors.append(copy)
return errors

Expand All @@ -924,23 +926,18 @@ def copy(
excluded_descendants=None,
**kwargs
):
try:
target, position = self.validate_targeting_args(target, position)
except ValidationError as e:
return str(e)
target, position = self.validate_targeting_args(target, position)

try:
source = self.get_queryset().get(pk=from_key)
except ContentNode.DoesNotExist:
error = ValidationError("Copy source node does not exist")
return str(error)
raise ValidationError("Copy source node does not exist")

# Affected channel for the copy is the target's channel
channel_id = target.channel_id

if ContentNode.filter_by_pk(pk=pk).exists():
error = ValidationError("Copy pk already exists")
return str(error)
raise ValidationError("Copy pk already exists")

can_edit_source_channel = ContentNode.filter_edit_queryset(
ContentNode.filter_by_pk(pk=source.id), user=self.request.user
Expand Down Expand Up @@ -969,8 +966,6 @@ def copy(
created_by_id=self.request.user.id,
)

return None

def perform_create(self, serializer, change=None):
instance = serializer.save()

Expand Down
10 changes: 7 additions & 3 deletions contentcuration/contentcuration/viewsets/sync/utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import logging

from django.conf import settings

from contentcuration.utils.sentry import report_exception
from contentcuration.viewsets.sync.constants import ALL_TABLES
from contentcuration.viewsets.sync.constants import CHANNEL
Expand Down Expand Up @@ -92,7 +94,9 @@ def log_sync_exception(e, user=None, change=None, changes=None):
elif changes is not None:
contexts["changes"] = changes

report_exception(e, user=user, contexts=contexts)
# in production, we'll get duplicates in Sentry if we log the exception here.
if settings.DEBUG:
# make sure we leave a record in the logs just in case.
logging.exception(e)

# make sure we leave a record in the logs just in case.
logging.exception(e)
report_exception(e, user=user, contexts=contexts)

0 comments on commit dfd6536

Please sign in to comment.