Initial migration/refactor of the build process to make usage of the Celery
handlers (`on_success`, `on_failure`, etc)

We use this environment a lot from our tests, which I don't like. However, for
now, I'm keeping the because the change is not small.

We should split unit-test from integration test and reduce the complexity of
executing "real commands" when calling tests in exchange of more mocked data
instead and unit tests for the logic.

Integration tests should not create the environment where to execute the
commands nor passing weird attributes to the task. They should mock the
responses of the API for the task to grab the correct data instead.

This way, we avoid having "extra arguments" on the tasks just for test purposes
and also avoid cheating the normal flow by overriding the environment with
custom attributes. This creates lot of complexity in our code and we endup with
multiple different flows that are not normal and won't ever happen in production
but that we need to maintain just for the tests.

In particular, we should avoid "integration tests" when they are not strictly
required. In this particular case, we could create a unit-test that checks the
logic for `SyncRepositoryMixin.validate_duplicate_reserved_versions` which is a
lot easier to generate the context required and then an "integration test" with
most things mocked (no `git` commands are executed at all) and only checks that
function is being called.

We are always using `CLEAN_AFTER_BUILD=True`, so there is no environment to wipe
anymore. This is a confusing message to send to the user since it has no effect.

I'm also removing the code because it doesn't make sense anymore. If we ever
need it again, we can re-implement it correctly and recover it from here.

We are not caching pip packages anymore.

There are lot of tests not really accurate and very complex without reason. I'd
like to use the same flow in production than in tests instead of
short-circuiting/hacking the process in tests to behave as we want.

We should use mocks to avoid executing commands when running tests instead, but
making sure the "build process" is following exact the same flow in both
environments. If we want to test something in particular outside the flow, we
should create unit tests for those functions/methods.

This way we avoid doing an aditional API call to get the version, since we
already have it in memory.

I wasn't able to mock the whole task as I wanted yet, but I follow a similar
convention than the one used for the migrated test. However, I've simplify them
a little.

There is no need to have a `base.py` and `BuildTaskBase` because it don't share
code with sync repository task.

Skip moving the file if the old artifact path does not exist.

Ideally, we should be able to mock:

* API calls
* Commands run inside the environment
* Git repository
* Read the Docs config file

This commit mocks most of them. The only missing here is the config file, but
there should be code already for that.

These mocks allow us to use the exact same flow than in production (e.g. hitting
the API to get the Version/Build/Project object) and avoid short-circuiting it.
Once the "mocked build" is finished, we can assert all the calls to them.

That way, it executes the `on_success`, `on_failure`, etc.

New test to check that we are updating the build object via the API, we are
sending notifications and more when the build fails.

It checks all the API calls done to the webs, and that the functions we expect
to be called are effectively called.

Move tests from `test_celery.py` to the "new system" and remove the invalid ones.

It was used to "force a build" by omiting the environment cached on disk.
However, this does not makes sense anymore since we are not caching
anything (always CLEAN_BUILD=True) and we run just one build per instance.

We always want to record a Build into our database. However, there are
_particular_ commands that we don't want to record to not expose them to the
user.

The un-needed logic was removed to simplify the flow.

This was only to communicate `BuildEnvironment.__exit__` that it should not log
the exception as an error.

If the API fails when retriving the build, we should use just the `build_pk`
passed when triggered the task to report it back. However, the API may fail
again when hit, but at least it does not fails in our application code.

Show different messages depending on the error type:

- user error: something the user should be able to solve by themselves (e.g.
  wrong RTD config file)
- app error: nothing the user can do, it's an internal error that we known (e.g.
  can't connect to Docker API)
- unknown error: we don't know what happened. Log the exception and show a
  generic error to the user

We are not using that field at all.

When the work `Killed` appears in the output it could be OOM or timeout. We
are not sure. So, we insert a generic message in the build's output saying that
could be one or the other.

However, if we are _sure_ what the error was, the Exception risen will have the
proper message and it will be shown correctly to the user.

Store the `Build.builder` in `before_start` so it's saved immediately next time
the Build object is updated instead of at the end of the build process.

`BaseVCS` class is raising a `RepositoryError` exception when the command
executed in the environment fails. The logic is as follows:

- Environment runs the command
- Raises a BuildUserError if the command fails
- `BaseVCS` detects this exception and re-raise a `RepositoryError`

Each VCS Backend class, handle this `RepositoryError` nicely and return whatever
is required in that case.

…humitos/celery-handlers

We need to re-define this task here to avoid old web instances trigger these
tasks while new builders won't have it configured.

Probably, we will want to deploy builders first.

Since we are using only 1 process per builder, there is no need to have a
mechanisim to check if there is another build running in the host and lock the
version.

This commit removes all the locking mechanisim to simplify the code and reduce
complexity. We can come back to this if we need it again in the future.

These fields were used long time ago and are not required anymore. We can go a
little further in the future and remove the database fields as well.

Co-authored-by: Anthony <aj@ohess.org>

…thedocs.org into humitos/celery-handlers

I thought they weren't required because, but they are. It could happen that
`fileify` tasks get queued and while we are deploying webs. In that case, when
new web instances spun up they won't have the old name defined.

So, it's a good idea to define the old name for these tasks as well.

- sync_repository_task: must receive `version_id`
- update_docs_task: must receive `version_id` and `build_id` and optionally a `build_commit`

Hopefully with this it doesn't exit with `Exited with code 137`

…humitos/celery-handlers

This allows new builds to execute tasks triggered by old web instances while
doing the deploy.

This code can be removed after that.

Use a `Task.data` (`readthedocs.projects.task.builds.TaskData` object) to store
all the data the task needs to work instead of storing it directly using
`self.`.

This is to allow us a simpler way to perform a clean _before_ (and/or _after_)
starting the execution of a new task and avoid potentially sharing state with a
previous task executed that may not be able to perform the cleanup.

The only thing we need to keep in mind is that when modifying these Celery
tasks, we _always_ have to add any new value inside the
`self.data.<my-new-attribute>` and not directly `self.<my-new-attribute>` to
avoid this problem. In the future, we could implement this protection at a code
level if we want to avoid this mistake.

See #8815 (comment)
See https://docs.celeryproject.org/en/master/userguide/tasks.html#instantiation

…humitos/celery-handlers

…thedocs.org into humitos/celery-handlers-self

…humitos/celery-handlers

I wasn't able to find nor solve the problem with the `cyclic-import` error
reported by pylint. So, I tried moving the utils functions used by search tasks
out from `readthedocs.projects.tasks.utils` into
`readthedocs.projects.tasks.search` instead.

This made `prospector` and `pylint` to pass locally without reporting any issue.

…thedocs.org into humitos/celery-handlers-self