Refactor tasks files

[humitos]

We have a lot of lines of code in projects/tasks.py and maybe some of them doesn't belong there or they can be moved to core/tasks.py.

Tasks we can consider moving,

• fileify
• send_notifications
• email_notification
• webhook_notification
• remove_dir
• clear_artifcats
• clear_*_artifacts
• finish_inactive_builds

We want to refactor these files and take a look if there are other task files than can be refactored also.

Ref: #3762

[shubham76]

Hey! Can I take this issue?

[RichardLitt]

@shubham76 Go ahead! Make sure to link back to this issue in your PR (I normally do this in the body), especially if the PR only affects one of the subtasks and not the entire issue.

[agjohnson]

This probably needs a little more hashing out before someone can jump in.

[agjohnson]

I raised this on the underlying PR comment already.

readthedocs.core is not an obvious spot for code, it doesn't mean much to me and has become a dumping ground for code. I'd prefer we look into a different pattern for moving these files around:

• Break up readthedocs.projects.tasks into submodules: readthedocs.projects.tasks.build or readthedocs.projects.tasks.sync, etc
• Move to an application where the tasks makes more sense -- if a task operates on versions, it should be in readthedocs.builds.tasks

I'd prefer either over readthedocs.core

[shubham76]

Okay! So we shouldn't move the code to readthedocs.core and instead to other submodules as you are suggesting.
In that case, I think I have messed up the PR. I'll start from scratch and try making changes as you had suggested here.

[shubham76]

Hey! @agjohnson So how many submodules should we have to move above mentioned files into?

[Alig1493]

Is this issue still open?

[stsewd]

@Alig1493 there is an open PR for this #3943

[ericholscher]

I closed #3943 because it is old and had many conflicts. If anyone else wants to attempt this, I'm all for it. Please do try and keep the refactor minimal though!

[Alig1493]

@ericholscher If this requires a new PR I'd like to take a stab at it.

[stsewd]

@Alig1493 go ahead!

[Alig1493]

@stsewd Before proceeding I noticed that there are 5 failing tests in the updated upstream master, namely in rtd_tests/tests/test_backend.py(3) and rtd_tests/tests/test_celery.py(2) files. Will this be an issue moving forward?

[stsewd]

@Alig1493 build is passing on master

[Alig1493]

@stsewd I keep getting errors in tests which require for me to connect to a remote repository. Is there any chance that there certain tests that will fail in local?

[dojutsu-user]

@Alig1493
I have just run all the tests and all are passing in local.

[Alig1493]

@dojutsu-user I see. I can't seem to figure out the issue on my end though. Keeps telling me it has failed to connect to remote repositories.

[stsewd]

Check that you have all the requirements for rtd, I guess it's your git version, it should be >=2.17

[Alig1493]

@stsewd My git version shows to be at version 2.7.4

[dojutsu-user]

@Alig1493 Can you post an screenshot or some error log??
and also, are you using tox?

[Alig1493]

@dojutsu-user sure.
And yes I'm using tox

[humitos]

@Alig1493 you are not using the correct git version. The first line of your screenshot says that --prune-tags arg is not known.

[shivanshu1234]

Hello, I see this issue still exists. I'm new here and would like to take a shot at it.

May I know why #4888 was not merged?

[stsewd]

It got staled and with a lot of conflicts I think

[shivanshu1234]

Okay, thank you.

[Iamshankhadeep]

is this issue still open?

[dojutsu-user]

Hi @Iamshankhadeep
Thanks for your interest in contribution.
There's already PR open for this issue #5333.
I would suggest to please look for another open issues.
Thanks.

[humitos]

Our code changed a lot since this issue was opened and we are going to remove a lot of this code in #6535 or a similar one, these days.