Improve task callable type hints

[gdemonet]

We disabled a pylint rule in #1477 to cope with type hints indicating a Optional[TaskError] return value. It seems we can avoid this altogether by adjusting the type hints, as explained below:

Going back to that, I still think it's not needed.

I think we may have messed up our type annotations.

Seeing the examples on this issue python/mypy#3157, one can see that the decorated function must be annotated with what it really returns (so if our function returns nothing, the return type must be None), because the decorator can have its own signature.

So in our case:

• __call__ always return None¹
• the decorator returns a function that return Optional[TaskError]

With the following diff, both pylint and mypy are happy and we don't need to disable anything.

diff --git a/.pylintrc b/.pylintrc
index ce7a5299..c4aae53d 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -47,7 +47,6 @@ disable=abstract-method,     # Too many false positive :/
         bad-continuation,    # I kindly disagree :)
         locally-disabled,    # Sometime we need to disable a warning locally.
         suppressed-message,  # Don't pollute the output with useless info.
-        useless-return       # Disagreement with mypy, see https://github.com/python/mypy/issues/3974
 
 # }}}
 # Basic {{{
diff --git a/buildchain/buildchain/docker_command.py b/buildchain/buildchain/docker_command.py
index 56e53c71..4cf26444 100644
--- a/buildchain/buildchain/docker_command.py
+++ b/buildchain/buildchain/docker_command.py
@@ -140,7 +140,7 @@ class DockerBuild:
 
     @task_error(docker.errors.BuildError, handler=build_error_handler)
     @task_error(docker.errors.APIError)
-    def __call__(self) -> Optional[TaskError]:
+    def __call__(self) -> None:
         DOCKER_CLIENT.images.build(
             tag=self.tag,
             path=self.path,
@@ -148,7 +148,6 @@ class DockerBuild:
             buildargs=self.buildargs,
             forcerm=True,
         )
-        return None
 
 
 class DockerRun:
@@ -266,14 +265,13 @@ class DockerRun:
     @task_error(docker.errors.ContainerError, handler=container_error_handler)
     @task_error(docker.errors.ImageNotFound)
     @task_error(docker.errors.APIError)
-    def __call__(self) -> Optional[TaskError]:
+    def __call__(self) -> None:
         run_config = self.expand_config()
         DOCKER_CLIENT.containers.run(
             image=self.builder.tag,
             command=self.command,
             **run_config
         )
-        return None
 
 
 class DockerTag:
@@ -294,10 +292,9 @@ class DockerTag:
 
     @task_error(docker.errors.BuildError, handler=build_error_handler)
     @task_error(docker.errors.APIError)
-    def __call__(self) -> Optional[TaskError]:
+    def __call__(self) -> None:
         to_tag = DOCKER_CLIENT.images.get(self.full_name)
         to_tag.tag(self.repository, tag=self.version)
-        return None
 
 
 class DockerPull:
@@ -338,7 +335,6 @@ class DockerPull:
                     s=self, observed_digest=pulled.id
                 )
             )
-
         return None
 
 
@@ -358,12 +354,10 @@ class DockerSave:
 
     @task_error(docker.errors.APIError)
     @task_error(OSError)
-    def __call__(self) -> Optional[TaskError]:
+    def __call__(self) -> None:
         to_save = DOCKER_CLIENT.images.get(self.tag)
         image_stream = to_save.save(named=True)
 
         with self.save_path.open('wb') as image_file:
             for chunk in image_stream:
                 image_file.write(chunk)
-
-        return None

¹: except the one you modified to check the digest that explicitly returns a task error (we may want to raise an exception instead and use task_error for that, to be consistent).

Originally posted by @slaperche-scality in #1477