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 c26a13c562
commit d3f6ca24c8
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
Raises:
RuntimeException: a local function error occurs
RuntimeError: a local function error occurs
Examples:
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.
* 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).
* 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.
* 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:

View File

@ -73,6 +73,9 @@ class ApplicationPackages(ApplicationProperties):
Args:
source(str): path to local directory
Raises:
UnknownPackageError: if specified package is unknown or doesn't exist
"""
local_dir = Path(source)
if not local_dir.is_dir():
@ -110,6 +113,9 @@ class ApplicationPackages(ApplicationProperties):
Args:
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?
# 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
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:
# 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
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)
@ -164,4 +164,4 @@ class Handler:
ExitCode: if result is empty and check is enabled
"""
if enabled and predicate:
raise ExitCode()
raise ExitCode

View File

@ -81,7 +81,7 @@ class Search(Handler):
list[AURPackage]: sorted list for packages
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:
raise OptionError(sort_by)

View File

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

View File

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

View File

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

View File

@ -48,8 +48,11 @@ class OfficialSyncdb(Official):
Returns:
AURPackage: package which match the package name
Raises:
UnknownPackageError: package doesn't exist
"""
try:
return next(AURPackage.from_pacman(package) for package in pacman.package_get(package_name))
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
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)
try:

View File

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

View File

@ -118,6 +118,9 @@ class RemotePush(LazyLogging):
Args:
result(Result): build result
Raises:
GitRemoteError: push processing error
"""
try:
with TemporaryDirectory(ignore_cleanup_errors=True) as dir_name:
@ -127,4 +130,4 @@ class RemotePush(LazyLogging):
commit_author=self.commit_author)
except Exception:
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
Raises:
ReportFailed: in case of any report unmatched exception
ReportError: in case of any report unmatched exception
"""
try:
self.generate(packages, result)
except Exception:
self.logger.exception("report generation failed")
raise ReportError()
raise ReportError

View File

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

View File

@ -133,12 +133,12 @@ class Watcher(LazyLogging):
tuple[Package, BuildStatus]: package and its status
Raises:
UnknownPackage: if no package found
UnknownPackageError: if no package found
"""
try:
return self.known[package_base]
except KeyError:
raise UnknownPackageError(package_base)
raise UnknownPackageError(package_base) from 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
Raises:
UnknownPackage: if no package found
UnknownPackageError: if no package found
"""
if package is None:
try:
package, _ = self.known[package_base]
except KeyError:
raise UnknownPackageError(package_base)
raise UnknownPackageError(package_base) from None
full_status = BuildStatus(status)
self.known[package_base] = (package, full_status)
self.database.package_update(package, full_status)

View File

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

View File

@ -158,13 +158,13 @@ class TriggerLoader(LazyLogging):
ModuleType: module loaded from the imported module
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)
try:
return import_module(package)
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:
"""
@ -179,7 +179,7 @@ class TriggerLoader(LazyLogging):
Trigger: loaded trigger based on settings
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)
try:
@ -200,7 +200,7 @@ class TriggerLoader(LazyLogging):
type[Trigger]: loaded trigger type by module path
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_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
Raises:
SyncFailed: in case of any synchronization unmatched exception
SynchronizationError: in case of any synchronization unmatched exception
"""
try:
self.sync(path, built_packages)
except Exception:
self.logger.exception("remote sync failed")
raise SynchronizationError()
raise SynchronizationError
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
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:
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
Raises:
InvalidOption: if size is more than 1TiB
OptionError: if size is more than 1TiB
"""
def str_level() -> str:
if level == 0:

View File

@ -258,7 +258,7 @@ class Package(LazyLogging):
Self: package properties
Raises:
InvalidPackageInfo: if there are parsing errors
PackageInfoError: if there are parsing errors
"""
srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path)
srcinfo, errors = parse_srcinfo(srcinfo_source)
@ -360,6 +360,9 @@ class Package(LazyLogging):
Returns:
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``
Raises:
PackageInfoError: if there are parsing errors
"""
srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path)
srcinfo, errors = parse_srcinfo(srcinfo_source)
@ -396,7 +399,7 @@ class Package(LazyLogging):
set[str]: list of package supported architectures
Raises:
InvalidPackageInfo: if there are parsing errors
PackageInfoError: if there are parsing errors
"""
srcinfo_source = Package._check_output("makepkg", "--printsrcinfo", cwd=path)
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
Raises:
InvalidPackageInfo: if there are parsing errors
PackageInfoError: if there are parsing errors
"""
if not self.is_vcs:
return self.version

View File

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

View File

@ -104,7 +104,7 @@ class Result:
Result: updated instance
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():
if base in self._success:

View File

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

View File

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

View File

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

View File

@ -113,9 +113,10 @@ class UploadView(BaseView):
Raises:
HTTPBadRequest: if bad data is supplied
HTTPCreated: on success response
HTTPNotFound: method is disabled by configuration
"""
if not self.configuration.getboolean("web", "enable_archive_upload", fallback=False):
raise HTTPNotFound()
raise HTTPNotFound
try:
reader = await self.request.multipart()
@ -141,4 +142,4 @@ class UploadView(BaseView):
target_location = current_location.parent / filename
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"]
self.service.logs_remove(package_base, None)
raise HTTPNoContent()
raise HTTPNoContent
@aiohttp_apispec.docs(
tags=["Packages"],
@ -97,7 +97,7 @@ class LogsView(BaseView):
try:
_, status = self.service.package_get(package_base)
except UnknownPackageError:
raise HTTPNotFound()
raise HTTPNotFound
logs = self.service.logs_get(package_base)
response = {
@ -143,4 +143,4 @@ class LogsView(BaseView):
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"]
self.service.package_remove(package_base)
raise HTTPNoContent()
raise HTTPNoContent
@aiohttp_apispec.docs(
tags=["Packages"],
@ -98,7 +98,7 @@ class PackageView(BaseView):
try:
package, status = self.service.package_get(package_base)
except UnknownPackageError:
raise HTTPNotFound()
raise HTTPNotFound
response = [
{
@ -146,4 +146,4 @@ class PackageView(BaseView):
except UnknownPackageError:
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()
raise HTTPNoContent()
raise HTTPNoContent

View File

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

View File

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

View File

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