Skip to content
This repository has been archived by the owner on Mar 23, 2021. It is now read-only.

Fix logging in migration task #14

Merged
merged 1 commit into from
Sep 15, 2017

Conversation

rudigiesler
Copy link
Collaborator

Currently, for a few of our logs in the celery task, we're referencing values on the migration object. But when we're updating the object, we're updating it in an atomic way, by doing a select + update. This has the side effect that the local django object doesn't get updated, so we are logging the old value on the migrate object.

This PR fixes this issue.

@dubesoftware
Copy link

dubesoftware commented Sep 14, 2017

Just wondering why the MigrateSubscription object is declared and assigned several times and not declared once at the top of the function and then referenced further down. Does model.save() return None in our current Django version?

@rudigiesler
Copy link
Collaborator Author

@dubesoftware The issue here is that it's possible at any time for a user to click the Cancel button while the task is running, so whenever we update the model status, we need to do it in a locking/atomic way.

I've achieved this by doing a .filter(id=model.id, status=status_we_expect).update(status_to_change_to), which doesn't return the updated model, just the number of models updated. So if the number of models updated is 1, then we know that the status was what we expected, and has been updated. But if the number of models is 0, that means that the status of the model has changed, and that we should stop processing.

@rudigiesler rudigiesler merged commit 3831018 into develop Sep 15, 2017
@rudigiesler rudigiesler deleted the feature/GPE-351-fix-log-messages-in-tasks branch September 15, 2017 14:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants