constistent classmethod and staticmethod usage

General idea is to use classmethod for every constructor and
statismethod otherwise.
Also use self and cls whenever it's possible to call static and class
methods

src/ahriman/application/lock.py

@@ -80,7 +80,7 @@ class Lock:
         :param exc_tb: exception traceback if any
         :return: always False (do not suppress any exception)
         """
-        self.remove()
+        self.clear()
         status = BuildStatusEnum.Success if exc_val is None else BuildStatusEnum.Failed
         self.reporter.update_self(status)
         return False
@@ -96,6 +96,14 @@ class Lock:
         if current_uid != root_uid:
             raise UnsafeRun(current_uid, root_uid)
 
+    def clear(self) -> None:
+        """
+        remove lock file
+        """
+        if self.path is None:
+            return
+        self.path.unlink(missing_ok=True)
+
     def create(self) -> None:
         """
         create lock file
@@ -106,11 +114,3 @@ class Lock:
             self.path.touch(exist_ok=self.force)
         except FileExistsError:
             raise DuplicateRun()
-
-    def remove(self) -> None:
-        """
-        remove lock file
-        """
-        if self.path is None:
-            return
-        self.path.unlink(missing_ok=True)

src/ahriman/core/build_tools/task.py

@@ -123,4 +123,4 @@ class Task:
         if self.cache_path.is_dir():
             # no need to clone whole repository, just copy from cache first
             shutil.copytree(self.cache_path, git_path)
-        return Task.fetch(git_path, self.package.git_url)
+        return self.fetch(git_path, self.package.git_url)

src/ahriman/core/report/report.py

@@ -17,9 +17,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
+from __future__ import annotations
+
 import logging
 
-from typing import Iterable
+from typing import Iterable, Type
 
 from ahriman.core.configuration import Configuration
 from ahriman.core.exceptions import ReportFailed
@@ -45,30 +47,34 @@ class Report:
         self.architecture = architecture
         self.configuration = configuration
 
-    @staticmethod
+    @classmethod
-    def run(architecture: str, configuration: Configuration, target: str, packages: Iterable[Package]) -> None:
+    def load(cls: Type[Report], architecture: str, configuration: Configuration, target: str) -> Report:
         """
-        run report generation
+        load client from settings
         :param architecture: repository architecture
         :param configuration: configuration instance
         :param target: target to generate report (e.g. html)
-        :param packages: list of packages to generate report
+        :return: client according to current settings
         """
         provider = ReportSettings.from_option(target)
         if provider == ReportSettings.HTML:
             from ahriman.core.report.html import HTML
-            report: Report = HTML(architecture, configuration)
+            return HTML(architecture, configuration)
-        else:
+        return cls(architecture, configuration)  # should never happen
-            report = Report(architecture, configuration)
-
-        try:
-            report.generate(packages)
-        except Exception:
-            report.logger.exception(f"report generation failed for target {provider.name}")
-            raise ReportFailed()
 
     def generate(self, packages: Iterable[Package]) -> None:
         """
         generate report for the specified packages
         :param packages: list of packages to generate report
         """
+
+    def run(self, packages: Iterable[Package]) -> None:
+        """
+        run report generation
+        :param packages: list of packages to generate report
+        """
+        try:
+            self.generate(packages)
+        except Exception:
+            self.logger.exception("report generation failed")
+            raise ReportFailed()

src/ahriman/core/repository/executor.py

@@ -108,7 +108,8 @@ class Executor(Cleaner):
         if targets is None:
             targets = self.configuration.getlist("report", "target")
         for target in targets:
-            Report.run(self.architecture, self.configuration, target, self.packages())
+            runner = Report.load(self.architecture, self.configuration, target)
+            runner.run(self.packages())
 
     def process_sync(self, targets: Optional[Iterable[str]]) -> None:
         """
@@ -118,7 +119,8 @@ class Executor(Cleaner):
         if targets is None:
             targets = self.configuration.getlist("upload", "target")
         for target in targets:
-            Upload.run(self.architecture, self.configuration, target, self.paths.repository)
+            runner = Upload.load(self.architecture, self.configuration, target)
+            runner.run(self.paths.repository)
 
     def process_update(self, packages: Iterable[Path]) -> Path:
         """

src/ahriman/core/status/client.py

@@ -19,7 +19,7 @@
 #
 from __future__ import annotations
 
-from typing import List, Optional, Tuple
+from typing import List, Optional, Tuple, Type
 
 from ahriman.core.configuration import Configuration
 from ahriman.models.build_status import BuildStatus, BuildStatusEnum
@@ -31,6 +31,20 @@ class Client:
     base build status reporter client
     """
 
+    @classmethod
+    def load(cls: Type[Client], configuration: Configuration) -> Client:
+        """
+        load client from settings
+        :param configuration: configuration instance
+        :return: client according to current settings
+        """
+        host = configuration.get("web", "host", fallback=None)
+        port = configuration.getint("web", "port", fallback=None)
+        if host is not None and port is not None:
+            from ahriman.core.status.web_client import WebClient
+            return WebClient(host, port)
+        return cls()
+
     def add(self, package: Package, status: BuildStatusEnum) -> None:
         """
         add new package with status
@@ -109,18 +123,3 @@ class Client:
         :param package: current package properties
         """
         return self.add(package, BuildStatusEnum.Unknown)
-
-    @staticmethod
-    def load(configuration: Configuration) -> Client:
-        """
-        load client from settings
-        :param configuration: configuration instance
-        :return: client according to current settings
-        """
-        host = configuration.get("web", "host", fallback=None)
-        port = configuration.getint("web", "port", fallback=None)
-        if host is None or port is None:
-            return Client()
-
-        from ahriman.core.status.web_client import WebClient
-        return WebClient(host, port)

src/ahriman/core/upload/upload.py

@@ -17,9 +17,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
+from __future__ import annotations
+
 import logging
 
 from pathlib import Path
+from typing import Type
 
 from ahriman.core.configuration import Configuration
 from ahriman.core.exceptions import SyncFailed
@@ -44,29 +47,33 @@ class Upload:
         self.architecture = architecture
         self.config = configuration
 
-    @staticmethod
+    @classmethod
-    def run(architecture: str, configuration: Configuration, target: str, path: Path) -> None:
+    def load(cls: Type[Upload], architecture: str, configuration: Configuration, target: str) -> Upload:
         """
-        run remote sync
+        load client from settings
         :param architecture: repository architecture
         :param configuration: configuration instance
         :param target: target to run sync (e.g. s3)
-        :param path: local path to sync
+        :return: client according to current settings
         """
         provider = UploadSettings.from_option(target)
         if provider == UploadSettings.Rsync:
             from ahriman.core.upload.rsync import Rsync
-            upload: Upload = Rsync(architecture, configuration)
+            return Rsync(architecture, configuration)
-        elif provider == UploadSettings.S3:
+        if provider == UploadSettings.S3:
             from ahriman.core.upload.s3 import S3
-            upload = S3(architecture, configuration)
+            return S3(architecture, configuration)
-        else:
+        return cls(architecture, configuration)  # should never happen
-            upload = Upload(architecture, configuration)
 
+    def run(self, path: Path) -> None:
+        """
+        run remote sync
+        :param path: local path to sync
+        """
         try:
-            upload.sync(path)
+            self.sync(path)
         except Exception:
-            upload.logger.exception(f"remote sync failed for {provider.name}")
+            self.logger.exception("remote sync failed")
             raise SyncFailed()
 
     def sync(self, path: Path) -> None:

src/ahriman/models/build_status.py

@@ -63,7 +63,7 @@ class BuildStatus:
     """
     build status holder
     :ivar status: build status
-    :ivar _timestamp: build status update time
+    :ivar timestamp: build status update time
     """
 
     def __init__(self, status: Union[BuildStatusEnum, str, None] = None,

src/ahriman/models/package.py

@@ -156,6 +156,27 @@ class Package:
             aur_url=dump["aur_url"],
             packages=packages)
 
+    @classmethod
+    def load(cls: Type[Package], path: Union[Path, str], pacman: Pacman, aur_url: str) -> Package:
+        """
+        package constructor from available sources
+        :param path: one of path to sources directory, path to archive or package name/base
+        :param pacman: alpm wrapper instance (required to load from archive)
+        :param aur_url: AUR root url
+        :return: package properties
+        """
+        try:
+            maybe_path = Path(path)
+            if maybe_path.is_dir():
+                return cls.from_build(maybe_path, aur_url)
+            if maybe_path.is_file():
+                return cls.from_archive(maybe_path, pacman, aur_url)
+            return cls.from_aur(str(path), aur_url)
+        except InvalidPackageInfo:
+            raise
+        except Exception as e:
+            raise InvalidPackageInfo(str(e))
+
     @staticmethod
     def dependencies(path: Path) -> Set[str]:
         """
@@ -194,29 +215,6 @@ class Package:
         prefix = f"{epoch}:" if epoch else ""
         return f"{prefix}{pkgver}-{pkgrel}"
 
-    @staticmethod
-    def load(path: Union[Path, str], pacman: Pacman, aur_url: str) -> Package:
-        """
-        package constructor from available sources
-        :param path: one of path to sources directory, path to archive or package name/base
-        :param pacman: alpm wrapper instance (required to load from archive)
-        :param aur_url: AUR root url
-        :return: package properties
-        """
-        try:
-            maybe_path = Path(path)
-            if maybe_path.is_dir():
-                package: Package = Package.from_build(maybe_path, aur_url)
-            elif maybe_path.is_file():
-                package = Package.from_archive(maybe_path, pacman, aur_url)
-            else:
-                package = Package.from_aur(str(path), aur_url)
-            return package
-        except InvalidPackageInfo:
-            raise
-        except Exception as e:
-            raise InvalidPackageInfo(str(e))
-
     def actual_version(self, paths: RepositoryPaths) -> str:
         """
         additional method to handle VCS package versions

src/ahriman/models/package_description.py

@@ -65,7 +65,7 @@ class PackageDescription:
         :param path: path to package archive
         :return: package properties based on tarball
         """
-        return PackageDescription(
+        return cls(
             architecture=package.arch,
             archive_size=package.size,
             build_date=package.builddate,

src/ahriman/models/report_settings.py

@@ -20,6 +20,7 @@
 from __future__ import annotations
 
 from enum import Enum, auto
+from typing import Type
 
 from ahriman.core.exceptions import InvalidOption
 
@@ -32,13 +33,13 @@ class ReportSettings(Enum):
 
     HTML = auto()
 
-    @staticmethod
+    @classmethod
-    def from_option(value: str) -> ReportSettings:
+    def from_option(cls: Type[ReportSettings], value: str) -> ReportSettings:
         """
         construct value from configuration
         :param value: configuration value
         :return: parsed value
         """
         if value.lower() in ("html",):
-            return ReportSettings.HTML
+            return cls.HTML
         raise InvalidOption(value)

src/ahriman/models/sign_settings.py

@@ -20,6 +20,7 @@
 from __future__ import annotations
 
 from enum import Enum, auto
+from typing import Type
 
 from ahriman.core.exceptions import InvalidOption
 
@@ -34,15 +35,15 @@ class SignSettings(Enum):
     SignPackages = auto()
     SignRepository = auto()
 
-    @staticmethod
+    @classmethod
-    def from_option(value: str) -> SignSettings:
+    def from_option(cls: Type[SignSettings], value: str) -> SignSettings:
         """
         construct value from configuration
         :param value: configuration value
         :return: parsed value
         """
         if value.lower() in ("package", "packages", "sign-package"):
-            return SignSettings.SignPackages
+            return cls.SignPackages
         if value.lower() in ("repository", "sign-repository"):
-            return SignSettings.SignRepository
+            return cls.SignRepository
         raise InvalidOption(value)

src/ahriman/models/upload_settings.py

@@ -20,6 +20,7 @@
 from __future__ import annotations
 
 from enum import Enum, auto
+from typing import Type
 
 from ahriman.core.exceptions import InvalidOption
 
@@ -34,15 +35,15 @@ class UploadSettings(Enum):
     Rsync = auto()
     S3 = auto()
 
-    @staticmethod
+    @classmethod
-    def from_option(value: str) -> UploadSettings:
+    def from_option(cls: Type[UploadSettings], value: str) -> UploadSettings:
         """
         construct value from configuration
         :param value: configuration value
         :return: parsed value
         """
         if value.lower() in ("rsync",):
-            return UploadSettings.Rsync
+            return cls.Rsync
         if value.lower() in ("s3",):
-            return UploadSettings.S3
+            return cls.S3
         raise InvalidOption(value)

tests/ahriman/application/test_lock.py

@@ -15,14 +15,14 @@ def test_enter(lock: Lock, mocker: MockerFixture) -> None:
     must process with context manager
     """
     check_user_mock = mocker.patch("ahriman.application.lock.Lock.check_user")
-    remove_mock = mocker.patch("ahriman.application.lock.Lock.remove")
+    clear_mock = mocker.patch("ahriman.application.lock.Lock.clear")
     create_mock = mocker.patch("ahriman.application.lock.Lock.create")
     update_status_mock = mocker.patch("ahriman.core.status.client.Client.update_self")
 
     with lock:
         pass
     check_user_mock.assert_called_once()
-    remove_mock.assert_called_once()
+    clear_mock.assert_called_once()
     create_mock.assert_called_once()
     update_status_mock.assert_has_calls([
         mock.call(BuildStatusEnum.Building),
@@ -35,7 +35,7 @@ def test_exit_with_exception(lock: Lock, mocker: MockerFixture) -> None:
     must process with context manager in case if exception raised
     """
     mocker.patch("ahriman.application.lock.Lock.check_user")
-    mocker.patch("ahriman.application.lock.Lock.remove")
+    mocker.patch("ahriman.application.lock.Lock.clear")
     mocker.patch("ahriman.application.lock.Lock.create")
     update_status_mock = mocker.patch("ahriman.core.status.client.Client.update_self")
 
@@ -79,6 +79,34 @@ def test_check_user_unsafe(lock: Lock) -> None:
     lock.check_user()
 
 
+def test_clear(lock: Lock) -> None:
+    """
+    must remove lock file
+    """
+    lock.path = Path(tempfile.mktemp())
+    lock.path.touch()
+
+    lock.clear()
+    assert not lock.path.is_file()
+
+
+def test_clear_missing(lock: Lock) -> None:
+    """
+    must not fail on lock removal if file is missing
+    """
+    lock.path = Path(tempfile.mktemp())
+    lock.clear()
+
+
+def test_clear_skip(lock: Lock, mocker: MockerFixture) -> None:
+    """
+    must skip removal if no file set
+    """
+    unlink_mock = mocker.patch("pathlib.Path.unlink")
+    lock.clear()
+    unlink_mock.assert_not_called()
+
+
 def test_create(lock: Lock) -> None:
     """
     must create lock
@@ -121,31 +149,3 @@ def test_create_unsafe(lock: Lock) -> None:
 
     lock.create()
     lock.path.unlink()
-
-
-def test_remove(lock: Lock) -> None:
-    """
-    must remove lock file
-    """
-    lock.path = Path(tempfile.mktemp())
-    lock.path.touch()
-
-    lock.remove()
-    assert not lock.path.is_file()
-
-
-def test_remove_missing(lock: Lock) -> None:
-    """
-    must not fail on lock removal if file is missing
-    """
-    lock.path = Path(tempfile.mktemp())
-    lock.remove()
-
-
-def test_remove_skip(lock: Lock, mocker: MockerFixture) -> None:
-    """
-    must skip removal if no file set
-    """
-    unlink_mock = mocker.patch("pathlib.Path.unlink")
-    lock.remove()
-    unlink_mock.assert_not_called()

tests/ahriman/core/report/test_report.py

@@ -15,7 +15,7 @@ def test_report_failure(configuration: Configuration, mocker: MockerFixture) ->
     """
     mocker.patch("ahriman.core.report.html.HTML.generate", side_effect=Exception())
     with pytest.raises(ReportFailed):
-        Report.run("x86_64", configuration, ReportSettings.HTML.name, Path("path"))
+        Report.load("x86_64", configuration, ReportSettings.HTML.name).run(Path("path"))
 
 
 def test_report_html(configuration: Configuration, mocker: MockerFixture) -> None:
@@ -23,5 +23,5 @@ def test_report_html(configuration: Configuration, mocker: MockerFixture) -> Non
     must generate html report
     """
     report_mock = mocker.patch("ahriman.core.report.html.HTML.generate")
-    Report.run("x86_64", configuration, ReportSettings.HTML.name, Path("path"))
+    Report.load("x86_64", configuration, ReportSettings.HTML.name).run(Path("path"))
     report_mock.assert_called_once()

tests/ahriman/core/status/test_client.py

@@ -7,6 +7,22 @@ from ahriman.models.build_status import BuildStatusEnum
 from ahriman.models.package import Package
 
 
+def test_load_dummy_client(configuration: Configuration) -> None:
+    """
+    must load dummy client if no settings set
+    """
+    assert isinstance(Client.load(configuration), Client)
+
+
+def test_load_full_client(configuration: Configuration) -> None:
+    """
+    must load full client if no settings set
+    """
+    configuration.set("web", "host", "localhost")
+    configuration.set("web", "port", "8080")
+    assert isinstance(Client.load(configuration), WebClient)
+
+
 def test_add(client: Client, package_ahriman: Package) -> None:
     """
     must process package addition without errors
@@ -98,19 +114,3 @@ def test_set_unknown(client: Client, package_ahriman: Package, mocker: MockerFix
     client.set_unknown(package_ahriman)
 
     add_mock.assert_called_with(package_ahriman, BuildStatusEnum.Unknown)
-
-
-def test_load_dummy_client(configuration: Configuration) -> None:
-    """
-    must load dummy client if no settings set
-    """
-    assert isinstance(Client.load(configuration), Client)
-
-
-def test_load_full_client(configuration: Configuration) -> None:
-    """
-    must load full client if no settings set
-    """
-    configuration.set("web", "host", "localhost")
-    configuration.set("web", "port", "8080")
-    assert isinstance(Client.load(configuration), WebClient)

tests/ahriman/core/test_configuration.py

@@ -14,8 +14,8 @@ def test_from_path(mocker: MockerFixture) -> None:
     load_logging_mock = mocker.patch("ahriman.core.configuration.Configuration.load_logging")
     path = Path("path")
 
-    config = Configuration.from_path(path, "x86_64", True)
+    configuration = Configuration.from_path(path, "x86_64", True)
-    assert config.path == path
+    assert configuration.path == path
     read_mock.assert_called_with(path)
     load_includes_mock.assert_called_once()
     load_logging_mock.assert_called_once()

tests/ahriman/core/upload/test_upload.py

@@ -15,7 +15,7 @@ def test_upload_failure(configuration: Configuration, mocker: MockerFixture) ->
     """
     mocker.patch("ahriman.core.upload.rsync.Rsync.sync", side_effect=Exception())
     with pytest.raises(SyncFailed):
-        Upload.run("x86_64", configuration, UploadSettings.Rsync.name, Path("path"))
+        Upload.load("x86_64", configuration, UploadSettings.Rsync.name).run(Path("path"))
 
 
 def test_upload_rsync(configuration: Configuration, mocker: MockerFixture) -> None:
@@ -23,7 +23,7 @@ def test_upload_rsync(configuration: Configuration, mocker: MockerFixture) -> No
     must upload via rsync
     """
     upload_mock = mocker.patch("ahriman.core.upload.rsync.Rsync.sync")
-    Upload.run("x86_64", configuration, UploadSettings.Rsync.name, Path("path"))
+    Upload.load("x86_64", configuration, UploadSettings.Rsync.name).run(Path("path"))
     upload_mock.assert_called_once()
 
 
@@ -32,5 +32,5 @@ def test_upload_s3(configuration: Configuration, mocker: MockerFixture) -> None:
     must upload via s3
     """
     upload_mock = mocker.patch("ahriman.core.upload.s3.S3.sync")
-    Upload.run("x86_64", configuration, UploadSettings.S3.name, Path("path"))
+    Upload.load("x86_64", configuration, UploadSettings.S3.name).run(Path("path"))
     upload_mock.assert_called_once()

tests/ahriman/models/test_package.py

@@ -135,24 +135,6 @@ def test_from_json_view_3(package_tpacpi_bat_git: Package) -> None:
     assert Package.from_json(package_tpacpi_bat_git.view()) == package_tpacpi_bat_git
 
 
-def test_dependencies_with_version(mocker: MockerFixture, resource_path_root: Path) -> None:
-    """
-    must load correct list of dependencies with version
-    """
-    srcinfo = (resource_path_root / "models" / "package_yay_srcinfo").read_text()
-    mocker.patch("pathlib.Path.read_text", return_value=srcinfo)
-
-    assert Package.dependencies(Path("path")) == {"git", "go", "pacman"}
-
-
-def test_full_version() -> None:
-    """
-    must construct full version
-    """
-    assert Package.full_version("1", "r2388.d30e3201", "1") == "1:r2388.d30e3201-1"
-    assert Package.full_version(None, "0.12.1", "1") == "0.12.1-1"
-
-
 def test_load_from_archive(package_ahriman: Package, pyalpm_handle: MagicMock, mocker: MockerFixture) -> None:
     """
     must load package from package archive
@@ -198,6 +180,24 @@ def test_load_failure(package_ahriman: Package, pyalpm_handle: MagicMock, mocker
         Package.load(Path("path"), pyalpm_handle, package_ahriman.aur_url)
 
 
+def test_dependencies_with_version(mocker: MockerFixture, resource_path_root: Path) -> None:
+    """
+    must load correct list of dependencies with version
+    """
+    srcinfo = (resource_path_root / "models" / "package_yay_srcinfo").read_text()
+    mocker.patch("pathlib.Path.read_text", return_value=srcinfo)
+
+    assert Package.dependencies(Path("path")) == {"git", "go", "pacman"}
+
+
+def test_full_version() -> None:
+    """
+    must construct full version
+    """
+    assert Package.full_version("1", "r2388.d30e3201", "1") == "1:r2388.d30e3201-1"
+    assert Package.full_version(None, "0.12.1", "1") == "0.12.1-1"
+
+
 def test_actual_version(package_ahriman: Package, repository_paths: RepositoryPaths) -> None:
     """
     must return same actual_version as version is