move_files tasks depends on the Version from the db and fails

[humitos]

Similar to #4789, we pass the Version.pk to the move_files task. If the version is removed before the task is executed, the task fails because the row is not in the database anymore.

The problem is at this line,

https://github.com/rtfd/readthedocs.org/blob/3fbb0c1a7c94bcc979f54d67fad8dc94f74854c8/readthedocs/projects/tasks.py#L838

We should handle this exception properly and do not fail the task. We need to think this a little more, but I suppose that if there is no version in the database, there is nothing to move or do at that point. So, I think that just adding a try/except block with an log.warning will do the trick.

This happens frequently: https://sentry.io/read-the-docs/readthedocs-org/issues/235379529/?query=is:unresolved

[dojutsu-user]

@humitos Would it be okay if i take this issue?

I think you want something like this

try:
    version = Version.objects.get(pk=version_pk)
except Version.DoesNotExist:
    log.warning(f'Version with pk {version_pk} does not exist')

[humitos]

I just realize that this also happens at

https://github.com/rtfd/readthedocs.org/blob/dfc8fc9eba8dc9caae171ca0b3e8f6a71594e088/readthedocs/projects/tasks.py#L1085-L1087

probably for the same reason that the Version/Project is deleted manually by the user.

We should think about a generic way of handling this from all the tasks that depend on objects being on the database.

[dojutsu-user]

@humitos Probably we can implement custom get() method in the model manager of Version. I found it here
https://github.com/rtfd/readthedocs.org/blob/9b2b17c0fc603267b4b20f5923862c2db82602da/readthedocs/builds/managers.py#L17

in this, we can write our own get() method which may probably look something like this

from django.core.exceptions import ObjectDoesNotExist

def get(self, **kwargs):
    try:
        return super().get(**kwargs)
    except ObjectDoesNotExist:
        # warning or something else

[humitos]

Sounds good to me.

We may have to name it different, though. I'd like to be more explicit on the name for the use case: get_object_from_task or get_object_or_log. I'm not sure, but something like that.

We will also need to handle when the query returns None from the task, so we stop the task there without failing.

[dojutsu-user]

get_object_or_log sounds good to me.
I am not sure about what you meant from stop the task, do you mean to return None from the task like this:

@app.task(queue='web')
def task_name(version_pk, *args, **kwargs):
    # code
    version = Version.objects.get_object_or_log(pk=version_pk)
    if not version:
        return None
    # code

[humitos]

@dojutsu-user yes, something like that is what I have in mind