review exception raise

In some cases for better readability of logs, exceptions are now raised
without parent exception stacktrace. Also updated docs and contributing
guidelines
This commit is contained in:
Evgenii Alekseev 2023-08-20 17:03:46 +03:00
parent ad1c0051c4
commit 6530afbfc7
32 changed files with 92 additions and 48 deletions

View File

@ -46,7 +46,7 @@ Again, the most checks can be performed by `make check` command, though some add
int: result int: result
Raises: Raises:
RuntimeException: a local function error occurs RuntimeError: a local function error occurs
Examples: Examples:
Very informative example how to use this function, e.g.:: Very informative example how to use this function, e.g.::
@ -130,6 +130,12 @@ Again, the most checks can be performed by `make check` command, though some add
* Configuration interactions must go through `ahriman.core.configuration.Configuration` class instance. * Configuration interactions must go through `ahriman.core.configuration.Configuration` class instance.
* In case if class load requires some actions, it is recommended to create class method which can be used for class instantiating. * In case if class load requires some actions, it is recommended to create class method which can be used for class instantiating.
* The code must follow the exception safety, unless it is explicitly asked by end user. It means that most exceptions must be handled and printed to log, no other actions must be done (e.g. raising another exception). * The code must follow the exception safety, unless it is explicitly asked by end user. It means that most exceptions must be handled and printed to log, no other actions must be done (e.g. raising another exception).
* Exceptions without parameters should be raised without parentheses, e.g.:
```python
raise RuntimeError
```
* For the external command `ahriman.core.util.check_output` function must be used. * For the external command `ahriman.core.util.check_output` function must be used.
* Every temporary file/directory must be removed at the end of processing, no matter what. The `tempfile` module provides good ways to do it. * Every temporary file/directory must be removed at the end of processing, no matter what. The `tempfile` module provides good ways to do it.
* Import order must be the following: * Import order must be the following:

View File

@ -73,6 +73,9 @@ class ApplicationPackages(ApplicationProperties):
Args: Args:
source(str): path to local directory source(str): path to local directory
Raises:
UnknownPackageError: if specified package is unknown or doesn't exist
""" """
local_dir = Path(source) local_dir = Path(source)
if not local_dir.is_dir(): if not local_dir.is_dir():
@ -110,6 +113,9 @@ class ApplicationPackages(ApplicationProperties):
Args: Args:
source(str): remote URL of the package archive source(str): remote URL of the package archive
Raises:
UnknownPackageError: if specified package is unknown or doesn't exist
""" """
dst = self.repository.paths.packages / Path(source).name # URL is path, is not it? dst = self.repository.paths.packages / Path(source).name # URL is path, is not it?
# timeout=None to suppress pylint warns. Also suppress bandit warnings # timeout=None to suppress pylint warns. Also suppress bandit warnings

View File

@ -61,7 +61,7 @@ class Handler:
list[str]: list of architectures for which tree is created list[str]: list of architectures for which tree is created
Raises: Raises:
MissingArchitecture: if no architecture set and automatic detection is not allowed or failed MissingArchitectureError: if no architecture set and automatic detection is not allowed or failed
""" """
if not cls.ALLOW_AUTO_ARCHITECTURE_RUN and args.architecture is None: if not cls.ALLOW_AUTO_ARCHITECTURE_RUN and args.architecture is None:
# for some parsers (e.g. config) we need to run with specific architecture # for some parsers (e.g. config) we need to run with specific architecture
@ -118,7 +118,7 @@ class Handler:
int: 0 on success, 1 otherwise int: 0 on success, 1 otherwise
Raises: Raises:
MultipleArchitectures: if more than one architecture supplied and no multi architecture supported MultipleArchitecturesError: if more than one architecture supplied and no multi architecture supported
""" """
architectures = cls.architectures_extract(args) architectures = cls.architectures_extract(args)
@ -164,4 +164,4 @@ class Handler:
ExitCode: if result is empty and check is enabled ExitCode: if result is empty and check is enabled
""" """
if enabled and predicate: if enabled and predicate:
raise ExitCode() raise ExitCode

View File

@ -81,7 +81,7 @@ class Search(Handler):
list[AURPackage]: sorted list for packages list[AURPackage]: sorted list for packages
Raises: Raises:
InvalidOption: if search fields is not in list of allowed ones OptionError: if search fields is not in list of allowed ones
""" """
if sort_by not in Search.SORT_FIELDS: if sort_by not in Search.SORT_FIELDS:
raise OptionError(sort_by) raise OptionError(sort_by)

View File

@ -72,6 +72,9 @@ class Users(Handler):
Returns: Returns:
User: built user descriptor User: built user descriptor
Raises:
PasswordError: password input is invalid
""" """
def read_password() -> str: def read_password() -> str:
first_password = getpass.getpass() first_password = getpass.getpass()

View File

@ -106,14 +106,14 @@ class Lock(LazyLogging):
create lock file create lock file
Raises: Raises:
DuplicateRun: if lock exists and no force flag supplied DuplicateRunError: if lock exists and no force flag supplied
""" """
if self.path is None: if self.path is None:
return return
try: try:
self.path.touch(exist_ok=self.force) self.path.touch(exist_ok=self.force)
except FileExistsError: except FileExistsError:
raise DuplicateRunError() raise DuplicateRunError from None
def watch(self) -> None: def watch(self) -> None:
""" """

View File

@ -83,7 +83,7 @@ class AUR(Remote):
list[AURPackage]: list of parsed packages list[AURPackage]: list of parsed packages
Raises: Raises:
InvalidPackageInfo: for error API response PackageInfoError: for error API response
""" """
response_type = response["type"] response_type = response["type"]
if response_type == "error": if response_type == "error":
@ -142,12 +142,15 @@ class AUR(Remote):
Returns: Returns:
AURPackage: package which match the package name AURPackage: package which match the package name
Raises:
UnknownPackageError: package doesn't exist
""" """
packages = self.make_request("info", package_name) packages = self.make_request("info", package_name)
try: try:
return next(package for package in packages if package.name == package_name) return next(package for package in packages if package.name == package_name)
except StopIteration: except StopIteration:
raise UnknownPackageError(package_name) raise UnknownPackageError(package_name) from None
def package_search(self, *keywords: str, pacman: Pacman) -> list[AURPackage]: def package_search(self, *keywords: str, pacman: Pacman) -> list[AURPackage]:
""" """

View File

@ -85,7 +85,7 @@ class Official(Remote):
list[AURPackage]: list of parsed packages list[AURPackage]: list of parsed packages
Raises: Raises:
InvalidPackageInfo: for error API response PackageInfoError: for error API response
""" """
if not response["valid"]: if not response["valid"]:
raise PackageInfoError("API validation error") raise PackageInfoError("API validation error")
@ -127,12 +127,15 @@ class Official(Remote):
Returns: Returns:
AURPackage: package which match the package name AURPackage: package which match the package name
Raises:
UnknownPackageError: package doesn't exist
""" """
packages = self.make_request(package_name, by="name") packages = self.make_request(package_name, by="name")
try: try:
return next(package for package in packages if package.name == package_name) return next(package for package in packages if package.name == package_name)
except StopIteration: except StopIteration:
raise UnknownPackageError(package_name) raise UnknownPackageError(package_name) from None
def package_search(self, *keywords: str, pacman: Pacman) -> list[AURPackage]: def package_search(self, *keywords: str, pacman: Pacman) -> list[AURPackage]:
""" """

View File

@ -48,8 +48,11 @@ class OfficialSyncdb(Official):
Returns: Returns:
AURPackage: package which match the package name AURPackage: package which match the package name
Raises:
UnknownPackageError: package doesn't exist
""" """
try: try:
return next(AURPackage.from_pacman(package) for package in pacman.package_get(package_name)) return next(AURPackage.from_pacman(package) for package in pacman.package_get(package_name))
except StopIteration: except StopIteration:
raise UnknownPackageError(package_name) raise UnknownPackageError(package_name) from None

View File

@ -81,7 +81,7 @@ class OAuth(Mapping):
type[aioauth_client.OAuth2Client]: loaded provider type type[aioauth_client.OAuth2Client]: loaded provider type
Raises: Raises:
InvalidOption: in case if invalid OAuth provider name supplied OptionError: in case if invalid OAuth provider name supplied
""" """
provider: type[aioauth_client.OAuth2Client] = getattr(aioauth_client, name) provider: type[aioauth_client.OAuth2Client] = getattr(aioauth_client, name)
try: try:

View File

@ -104,9 +104,12 @@ class RemotePull(LazyLogging):
def run(self) -> None: def run(self) -> None:
""" """
run git pull action run git pull action
Raises:
GitRemoteError: pull processing error
""" """
try: try:
self.repo_clone() self.repo_clone()
except Exception: except Exception:
self.logger.exception("git pull failed") self.logger.exception("git pull failed")
raise GitRemoteError() raise GitRemoteError

View File

@ -118,6 +118,9 @@ class RemotePush(LazyLogging):
Args: Args:
result(Result): build result result(Result): build result
Raises:
GitRemoteError: push processing error
""" """
try: try:
with TemporaryDirectory(ignore_cleanup_errors=True) as dir_name: with TemporaryDirectory(ignore_cleanup_errors=True) as dir_name:
@ -127,4 +130,4 @@ class RemotePush(LazyLogging):
commit_author=self.commit_author) commit_author=self.commit_author)
except Exception: except Exception:
self.logger.exception("git push failed") self.logger.exception("git push failed")
raise GitRemoteError() raise GitRemoteError

View File

@ -116,10 +116,10 @@ class Report(LazyLogging):
packages(list[Package]): list of packages to generate report packages(list[Package]): list of packages to generate report
Raises: Raises:
ReportFailed: in case of any report unmatched exception ReportError: in case of any report unmatched exception
""" """
try: try:
self.generate(packages, result) self.generate(packages, result)
except Exception: except Exception:
self.logger.exception("report generation failed") self.logger.exception("report generation failed")
raise ReportError() raise ReportError

View File

@ -89,6 +89,9 @@ class Telegram(Report, JinjaTemplate):
Args: Args:
packages(list[Package]): list of packages to generate report packages(list[Package]): list of packages to generate report
result(Result): build result result(Result): build result
Raises:
ValueError: impossible to split message by chunks
""" """
if not result.success: if not result.success:
return return

View File

@ -133,12 +133,12 @@ class Watcher(LazyLogging):
tuple[Package, BuildStatus]: package and its status tuple[Package, BuildStatus]: package and its status
Raises: Raises:
UnknownPackage: if no package found UnknownPackageError: if no package found
""" """
try: try:
return self.known[package_base] return self.known[package_base]
except KeyError: except KeyError:
raise UnknownPackageError(package_base) raise UnknownPackageError(package_base) from None
def package_remove(self, package_base: str) -> None: def package_remove(self, package_base: str) -> None:
""" """
@ -161,13 +161,13 @@ class Watcher(LazyLogging):
package(Package | None): optional package description. In case if not set current properties will be used package(Package | None): optional package description. In case if not set current properties will be used
Raises: Raises:
UnknownPackage: if no package found UnknownPackageError: if no package found
""" """
if package is None: if package is None:
try: try:
package, _ = self.known[package_base] package, _ = self.known[package_base]
except KeyError: except KeyError:
raise UnknownPackageError(package_base) raise UnknownPackageError(package_base) from None
full_status = BuildStatus(status) full_status = BuildStatus(status)
self.known[package_base] = (package, full_status) self.known[package_base] = (package, full_status)
self.database.package_update(package, full_status) self.database.package_update(package, full_status)

View File

@ -140,6 +140,9 @@ class KeyringGenerator(PkgbuildGenerator):
Args: Args:
source_path(Path): destination of the file content source_path(Path): destination of the file content
Raises:
PkgbuildGeneratorError: no trusted keys available
""" """
if not self.trusted: if not self.trusted:
raise PkgbuildGeneratorError raise PkgbuildGeneratorError

View File

@ -158,13 +158,13 @@ class TriggerLoader(LazyLogging):
ModuleType: module loaded from the imported module ModuleType: module loaded from the imported module
Raises: Raises:
InvalidExtension: in case if module cannot be loaded from specified package ExtensionError: in case if module cannot be loaded from specified package
""" """
self.logger.info("load module from package %s", package) self.logger.info("load module from package %s", package)
try: try:
return import_module(package) return import_module(package)
except ModuleNotFoundError: except ModuleNotFoundError:
raise ExtensionError(f"Module {package} not found") raise ExtensionError(f"Module {package} not found") from None
def load_trigger(self, module_path: str, architecture: str, configuration: Configuration) -> Trigger: def load_trigger(self, module_path: str, architecture: str, configuration: Configuration) -> Trigger:
""" """
@ -179,7 +179,7 @@ class TriggerLoader(LazyLogging):
Trigger: loaded trigger based on settings Trigger: loaded trigger based on settings
Raises: Raises:
InvalidExtension: in case if trigger could not be instantiated ExtensionError: in case if trigger could not be instantiated
""" """
trigger_type = self.load_trigger_class(module_path) trigger_type = self.load_trigger_class(module_path)
try: try:
@ -200,7 +200,7 @@ class TriggerLoader(LazyLogging):
type[Trigger]: loaded trigger type by module path type[Trigger]: loaded trigger type by module path
Raises: Raises:
InvalidExtension: in case if module cannot be loaded from the specified module path or is not a trigger ExtensionError: in case if module cannot be loaded from the specified module path or is not a trigger
""" """
*package_path_parts, class_name = module_path.split(".") *package_path_parts, class_name = module_path.split(".")
package_or_path = ".".join(package_path_parts) package_or_path = ".".join(package_path_parts)

View File

@ -104,13 +104,13 @@ class Upload(LazyLogging):
built_packages(list[Package]): list of packages which has just been built built_packages(list[Package]): list of packages which has just been built
Raises: Raises:
SyncFailed: in case of any synchronization unmatched exception SynchronizationError: in case of any synchronization unmatched exception
""" """
try: try:
self.sync(path, built_packages) self.sync(path, built_packages)
except Exception: except Exception:
self.logger.exception("remote sync failed") self.logger.exception("remote sync failed")
raise SynchronizationError() raise SynchronizationError
def sync(self, path: Path, built_packages: list[Package]) -> None: def sync(self, path: Path, built_packages: list[Package]) -> None:
""" """

View File

@ -160,7 +160,7 @@ def check_user(paths: RepositoryPaths, *, unsafe: bool) -> None:
unsafe(bool): if set no user check will be performed before path creation unsafe(bool): if set no user check will be performed before path creation
Raises: Raises:
UnsafeRun: if root uid differs from current uid and check is enabled UnsafeRunError: if root uid differs from current uid and check is enabled
Examples: Examples:
Simply run function with arguments:: Simply run function with arguments::
@ -345,7 +345,7 @@ def pretty_size(size: float | None, level: int = 0) -> str:
str: pretty printable size as string str: pretty printable size as string
Raises: Raises:
InvalidOption: if size is more than 1TiB OptionError: if size is more than 1TiB
""" """
def str_level() -> str: def str_level() -> str:
if level == 0: if level == 0:

View File

@ -258,7 +258,7 @@ class Package(LazyLogging):
Self: package properties Self: package properties
Raises: Raises:
InvalidPackageInfo: if there are parsing errors PackageInfoError: if there are parsing errors
""" """
srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path) srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path)
srcinfo, errors = parse_srcinfo(srcinfo_source) srcinfo, errors = parse_srcinfo(srcinfo_source)
@ -360,6 +360,9 @@ class Package(LazyLogging):
Returns: Returns:
Generator[Path, None, None]: list of paths of files which belong to the package and distributed together Generator[Path, None, None]: list of paths of files which belong to the package and distributed together
with this tarball. All paths are relative to the ``path`` with this tarball. All paths are relative to the ``path``
Raises:
PackageInfoError: if there are parsing errors
""" """
srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path) srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path)
srcinfo, errors = parse_srcinfo(srcinfo_source) srcinfo, errors = parse_srcinfo(srcinfo_source)
@ -396,7 +399,7 @@ class Package(LazyLogging):
set[str]: list of package supported architectures set[str]: list of package supported architectures
Raises: Raises:
InvalidPackageInfo: if there are parsing errors PackageInfoError: if there are parsing errors
""" """
srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path) srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path)
srcinfo, errors = parse_srcinfo(srcinfo_source) srcinfo, errors = parse_srcinfo(srcinfo_source)
@ -435,7 +438,7 @@ class Package(LazyLogging):
str: package version if package is not VCS and current version according to VCS otherwise str: package version if package is not VCS and current version according to VCS otherwise
Raises: Raises:
InvalidPackageInfo: if there are parsing errors PackageInfoError: if there are parsing errors
""" """
if not self.is_vcs: if not self.is_vcs:
return self.version return self.version

View File

@ -166,7 +166,7 @@ class RepositoryPaths:
path(Path): path to be chown path(Path): path to be chown
Raises: Raises:
InvalidPath: if path does not belong to root PathError: if path does not belong to root
""" """
def set_owner(current: Path) -> None: def set_owner(current: Path) -> None:
uid, gid = self.owner(current) uid, gid = self.owner(current)

View File

@ -104,7 +104,7 @@ class Result:
Result: updated instance Result: updated instance
Raises: Raises:
SuccessFailed: if there is previously failed package which is masked as success UnprocessedPackageStatusError: if there is previously failed package which is masked as success
""" """
for base, package in other._failed.items(): for base, package in other._failed.items():
if base in self._success: if base in self._success:

View File

@ -52,6 +52,9 @@ def exception_handler(logger: logging.Logger) -> MiddlewareType:
Returns: Returns:
MiddlewareType: built middleware MiddlewareType: built middleware
Raises:
HTTPNoContent: OPTIONS method response
""" """
@middleware @middleware
async def handle(request: Request, handler: HandlerType) -> StreamResponse: async def handle(request: Request, handler: HandlerType) -> StreamResponse:

View File

@ -121,7 +121,7 @@ class BaseView(View, CorsViewMixin):
if not value: if not value:
raise KeyError(key) raise KeyError(key)
except Exception: except Exception:
raise KeyError(f"Key {key} is missing or empty") raise KeyError(f"Key {key} is missing or empty") from None
return value return value
async def data_as_json(self, list_keys: list[str]) -> dict[str, Any]: async def data_as_json(self, list_keys: list[str]) -> dict[str, Any]:

View File

@ -74,7 +74,7 @@ class PGPView(BaseView):
try: try:
key = self.service.repository.sign.key_download(server, key) key = self.service.repository.sign.key_download(server, key)
except Exception: except Exception:
raise HTTPNotFound() raise HTTPNotFound
return json_response({"key": key}) return json_response({"key": key})

View File

@ -113,9 +113,10 @@ class UploadView(BaseView):
Raises: Raises:
HTTPBadRequest: if bad data is supplied HTTPBadRequest: if bad data is supplied
HTTPCreated: on success response HTTPCreated: on success response
HTTPNotFound: method is disabled by configuration
""" """
if not self.configuration.getboolean("web", "enable_archive_upload", fallback=False): if not self.configuration.getboolean("web", "enable_archive_upload", fallback=False):
raise HTTPNotFound() raise HTTPNotFound
try: try:
reader = await self.request.multipart() reader = await self.request.multipart()
@ -141,4 +142,4 @@ class UploadView(BaseView):
target_location = current_location.parent / filename target_location = current_location.parent / filename
current_location.rename(target_location) current_location.rename(target_location)
raise HTTPCreated() raise HTTPCreated

View File

@ -65,7 +65,7 @@ class LogsView(BaseView):
package_base = self.request.match_info["package"] package_base = self.request.match_info["package"]
self.service.logs_remove(package_base, None) self.service.logs_remove(package_base, None)
raise HTTPNoContent() raise HTTPNoContent
@aiohttp_apispec.docs( @aiohttp_apispec.docs(
tags=["Packages"], tags=["Packages"],
@ -97,7 +97,7 @@ class LogsView(BaseView):
try: try:
_, status = self.service.package_get(package_base) _, status = self.service.package_get(package_base)
except UnknownPackageError: except UnknownPackageError:
raise HTTPNotFound() raise HTTPNotFound
logs = self.service.logs_get(package_base) logs = self.service.logs_get(package_base)
response = { response = {
@ -143,4 +143,4 @@ class LogsView(BaseView):
self.service.logs_update(LogRecordId(package_base, version), created, record) self.service.logs_update(LogRecordId(package_base, version), created, record)
raise HTTPNoContent() raise HTTPNoContent

View File

@ -66,7 +66,7 @@ class PackageView(BaseView):
package_base = self.request.match_info["package"] package_base = self.request.match_info["package"]
self.service.package_remove(package_base) self.service.package_remove(package_base)
raise HTTPNoContent() raise HTTPNoContent
@aiohttp_apispec.docs( @aiohttp_apispec.docs(
tags=["Packages"], tags=["Packages"],
@ -98,7 +98,7 @@ class PackageView(BaseView):
try: try:
package, status = self.service.package_get(package_base) package, status = self.service.package_get(package_base)
except UnknownPackageError: except UnknownPackageError:
raise HTTPNotFound() raise HTTPNotFound
response = [ response = [
{ {
@ -146,4 +146,4 @@ class PackageView(BaseView):
except UnknownPackageError: except UnknownPackageError:
raise HTTPBadRequest(reason=f"Package {package_base} is unknown, but no package body set") raise HTTPBadRequest(reason=f"Package {package_base} is unknown, but no package body set")
raise HTTPNoContent() raise HTTPNoContent

View File

@ -88,4 +88,4 @@ class PackagesView(BaseView):
""" """
self.service.load() self.service.load()
raise HTTPNoContent() raise HTTPNoContent

View File

@ -104,4 +104,4 @@ class StatusView(BaseView):
self.service.status_update(status) self.service.status_update(status)
raise HTTPNoContent() raise HTTPNoContent

View File

@ -84,7 +84,7 @@ class LoginView(BaseView):
await remember(self.request, response, identity) await remember(self.request, response, identity)
raise response raise response
raise HTTPUnauthorized() raise HTTPUnauthorized
@aiohttp_apispec.docs( @aiohttp_apispec.docs(
tags=["Login"], tags=["Login"],
@ -114,4 +114,4 @@ class LoginView(BaseView):
await remember(self.request, response, identity) await remember(self.request, response, identity)
raise response raise response
raise HTTPUnauthorized() raise HTTPUnauthorized

View File

@ -57,6 +57,7 @@ class LogoutView(BaseView):
Raises: Raises:
HTTPFound: on success response HTTPFound: on success response
HTTPUnauthorized: no authorization cookie available
""" """
try: try:
await check_authorized(self.request) await check_authorized(self.request)