small improvements on code smell

* fix some grammar/typo errors
* change some statements to be more clear
* use pattern matching for enum processing
This commit is contained in:
Evgenii Alekseev 2023-08-23 18:02:45 +03:00
parent 477c473187
commit f6081507c0
32 changed files with 204 additions and 174 deletions

View File

@ -86,7 +86,7 @@ def _parser() -> argparse.ArgumentParser:
action="store_true")
parser.add_argument("--wait-timeout", help="wait for lock to be free. Negative value will lead to "
"immediate application run even if there is lock file. "
"In case of zero value, tthe application will wait infinitely",
"In case of zero value, the application will wait infinitely",
type=int, default=-1)
parser.add_argument("-V", "--version", action="version", version=__version__)

View File

@ -117,7 +117,6 @@ class ApplicationPackages(ApplicationProperties):
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
try:
response = requests.get(source, stream=True, timeout=None) # nosec
@ -125,6 +124,7 @@ class ApplicationPackages(ApplicationProperties):
except Exception:
raise UnknownPackageError(source)
dst = self.repository.paths.packages / Path(source).name # URL is path, is not it?
with dst.open("wb") as local_file:
for chunk in response.iter_content(chunk_size=1024):
local_file.write(chunk)

View File

@ -94,10 +94,13 @@ class Handler:
"""
try:
configuration = Configuration.from_path(args.configuration, architecture)
log_handler = Log.handler(args.log_handler)
Log.load(configuration, log_handler, quiet=args.quiet, report=args.report)
with Lock(args, architecture, configuration):
cls.run(args, architecture, configuration, report=args.report)
return True
except ExitCode:
return False
@ -128,8 +131,7 @@ class Handler:
raise MultipleArchitecturesError(args.command)
with Pool(len(architectures)) as pool:
result = pool.starmap(
cls.call, [(args, architecture) for architecture in architectures])
result = pool.starmap(cls.call, [(args, architecture) for architecture in architectures])
else:
result = [cls.call(args, architectures.pop())]

View File

@ -51,15 +51,16 @@ class Patch(Handler):
application = Application(architecture, configuration, report=report)
application.on_start()
if args.action == Action.Update and args.variable is not None:
match args.action:
case Action.Update if args.variable is not None:
patch = Patch.patch_create_from_function(args.variable, args.patch)
Patch.patch_set_create(application, args.package, patch)
elif args.action == Action.Update and args.variable is None:
case Action.Update:
package_base, patch = Patch.patch_create_from_diff(args.package, architecture, args.track)
Patch.patch_set_create(application, package_base, patch)
elif args.action == Action.List:
case Action.List:
Patch.patch_set_list(application, args.package, args.variable, args.exit_code)
elif args.action == Action.Remove:
case Action.Remove:
Patch.patch_set_remove(application, args.package, args.variable)
@staticmethod

View File

@ -48,7 +48,7 @@ class ServiceUpdates(Handler):
application = Application(architecture, configuration, report=report)
remote = Package.from_aur("ahriman", application.repository.pacman, None)
release = remote.version.rsplit("-", 1)[-1] # we don't store pkgrel locally, so we just append it
_, release = remote.version.rsplit("-", 1) # we don't store pkgrel locally, so we just append it
local_version = f"{__version__}-{release}"
# technically we would like to compare versions, but it is fine to raise an exception in case if locally

View File

@ -46,13 +46,14 @@ class StatusUpdate(Handler):
# we are using reporter here
client = Application(architecture, configuration, report=True).repository.reporter
if args.action == Action.Update and args.package:
match args.action:
case Action.Update if args.package:
# update packages statuses
for package in args.package:
client.package_update(package, args.status)
elif args.action == Action.Update:
case Action.Update:
# update service status
client.status_update(args.status)
elif args.action == Action.Remove:
case Action.Remove:
for package in args.package:
client.package_remove(package)

View File

@ -49,17 +49,18 @@ class Users(Handler):
"""
database = SQLite.load(configuration)
if args.action == Action.Update:
match args.action:
case Action.Update:
user = Users.user_create(args)
# if password is left blank we are not going to require salt to be set
salt = configuration.get("auth", "salt", fallback="") if user.password else ""
database.user_update(user.hash_password(salt))
elif args.action == Action.List:
case Action.List:
users = database.user_list(args.username, args.role)
Users.check_if_empty(args.exit_code, not users)
for user in users:
UserPrinter(user).print(verbose=True)
elif args.action == Action.Remove:
case Action.Remove:
database.user_remove(args.username)
@staticmethod

View File

@ -49,6 +49,16 @@ class Pacman(LazyLogging):
self.__create_handle_fn: Callable[[], Handle] = lambda: self.__create_handle(
architecture, configuration, refresh_database=refresh_database)
@cached_property
def handle(self) -> Handle:
"""
pyalpm handle
Returns:
Handle: generated pyalpm handle instance
"""
return self.__create_handle_fn()
def __create_handle(self, architecture: str, configuration: Configuration, *,
refresh_database: PacmanSynchronization) -> Handle:
"""
@ -79,16 +89,6 @@ class Pacman(LazyLogging):
return handle
@cached_property
def handle(self) -> Handle:
"""
pyalpm handle
Returns:
Handle: generated pyalpm handle instance
"""
return self.__create_handle_fn()
def database_copy(self, handle: Handle, database: DB, pacman_root: Path, paths: RepositoryPaths, *,
use_ahriman_cache: bool) -> None:
"""
@ -116,7 +116,7 @@ class Pacman(LazyLogging):
src = repository_database(pacman_root)
if not src.is_file():
self.logger.warning("repository %s is set to be used, however, no working copy was found", database.name)
return # database for some reasons deos not exist
return # database for some reason deos not exist
self.logger.info("copy pacman database from operating system root to ahriman's home")
shutil.copy(src, dst)
paths.chown(dst)

View File

@ -74,13 +74,14 @@ class Auth(LazyLogging):
Returns:
Auth: authorization module according to current settings
"""
provider = AuthSettings.from_option(configuration.get("auth", "target", fallback="disabled"))
if provider == AuthSettings.Configuration:
match AuthSettings.from_option(configuration.get("auth", "target", fallback="disabled")):
case AuthSettings.Configuration:
from ahriman.core.auth.mapping import Mapping
return Mapping(configuration, database)
if provider == AuthSettings.OAuth:
case AuthSettings.OAuth:
from ahriman.core.auth.oauth import OAuth
return OAuth(configuration, database)
case _:
return Auth(configuration)
async def check_credentials(self, username: str | None, password: str | None) -> bool:

View File

@ -28,8 +28,8 @@ from ahriman.models.auth_settings import AuthSettings
class OAuth(Mapping):
"""
OAuth's user authorization.
It is required to create application first and put application credentials.
User authorization implementation via OAuth. It is required to create application first and put application
credentials.
Attributes:
client_id(str): application client id

View File

@ -266,6 +266,11 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = {
"required": True,
"path_exists": True,
},
"timeout": {
"type": "integer",
"coerce": "integer",
"min": 0,
},
"unix_socket": {
"type": "path",
"coerce": "absolute_path",

View File

@ -136,12 +136,12 @@ class Validator(RootValidator):
The rule's arguments are validated against this schema:
{"type": "list", "schema": {"type": "string"}}
"""
url = urlparse(value) # it probably will never rise exceptions on parse
if not url.scheme:
match urlparse(value): # it probably will never rise exceptions on parse
case url if not url.scheme:
self._error(field, f"Url scheme is not set for {value}")
if not url.netloc and url.scheme not in ("file",):
case url if not url.netloc and url.scheme not in ("file",):
self._error(field, f"Location must be set for url {value} of scheme {url.scheme}")
if constraint and url.scheme not in constraint:
case url if constraint and url.scheme not in constraint:
self._error(field, f"Url {value} scheme must be one of {constraint}")
def _validate_path_exists(self, constraint: bool, field: str, value: Path) -> None:
@ -157,7 +157,8 @@ class Validator(RootValidator):
The rule's arguments are validated against this schema:
{"type": "boolean"}
"""
if constraint and not value.exists():
self._error(field, f"Path {value} must exist")
if not constraint and value.exists():
match value.exists():
case True if not constraint:
self._error(field, f"Path {value} must not exist")
case False if constraint:
self._error(field, f"Path {value} must exist")

View File

@ -80,22 +80,23 @@ class Report(LazyLogging):
Report: client according to current settings
"""
section, provider_name = configuration.gettype(target, architecture)
provider = ReportSettings.from_option(provider_name)
if provider == ReportSettings.HTML:
match ReportSettings.from_option(provider_name):
case ReportSettings.HTML:
from ahriman.core.report.html import HTML
return HTML(architecture, configuration, section)
if provider == ReportSettings.Email:
case ReportSettings.Email:
from ahriman.core.report.email import Email
return Email(architecture, configuration, section)
if provider == ReportSettings.Console:
case ReportSettings.Console:
from ahriman.core.report.console import Console
return Console(architecture, configuration, section)
if provider == ReportSettings.Telegram:
case ReportSettings.Telegram:
from ahriman.core.report.telegram import Telegram
return Telegram(architecture, configuration, section)
if provider == ReportSettings.RemoteCall:
case ReportSettings.RemoteCall:
from ahriman.core.report.remote_call import RemoteCall
return RemoteCall(architecture, configuration, section)
case _:
return Report(architecture, configuration) # should never happen
def generate(self, packages: list[Package], result: Result) -> None:

View File

@ -40,7 +40,7 @@ class Repository(Executor, UpdateHandler):
Examples:
This class along with traits provides access to local repository actions, e.g. remove packages, update packages,
sync local repository to remote, generate report, etc::
sync local repository to remote, generate report, etc.::
>>> from ahriman.core.configuration import Configuration
>>> from ahriman.core.database import SQLite

View File

@ -33,7 +33,7 @@ class GPG(SyncHttpClient):
Attributes:
configuration(Configuration): configuration instance
default_key(str | None): default PGP key ID to use
targets(set[SignSettings]): list of targets to sign (repository, package etc)
targets(set[SignSettings]): list of targets to sign (repository, package etc.)
"""
_check_output = check_output
@ -116,7 +116,7 @@ class GPG(SyncHttpClient):
download key from public PGP server
Args:
server(str): public PGP server which will be used to download the key
server(str): public PGP server which will be used to download data
key(str): key ID to download
Returns:
@ -163,7 +163,7 @@ class GPG(SyncHttpClient):
import key to current user and sign it locally
Args:
server(str): public PGP server which will be used to download the key
server(str): public PGP server which will be used to download data
key(str): key ID to import
"""
key_body = self.key_download(server, key)

View File

@ -113,7 +113,7 @@ class Watcher(LazyLogging):
Args:
log_record_id(LogRecordId): log record id
created(float): log created record
created(float): log created timestamp
record(str): log record
"""
if self._last_log_record_id != log_record_id:

View File

@ -92,7 +92,7 @@ class Tree:
dependency tree implementation
Attributes:
leaves[list[Leaf]): list of tree leaves
leaves(list[Leaf]): list of tree leaves
Examples:
The most important feature here is to generate tree levels one by one which can be achieved by using class

View File

@ -31,11 +31,11 @@ from ahriman.models.package import Package
class Github(HttpUpload):
"""
upload files to github releases
upload files to GitHub releases
Attributes:
github_owner(str): github repository owner
github_repository(str): github repository name
github_owner(str): GitHub repository owner
github_repository(str): GitHub repository name
"""
def __init__(self, architecture: str, configuration: Configuration, section: str) -> None:
@ -100,7 +100,7 @@ class Github(HttpUpload):
def files_remove(self, release: dict[str, Any], local_files: dict[Path, str], remote_files: dict[str, str]) -> None:
"""
remove files from github
remove files from GitHub
Args:
release(dict[str, Any]): release object
@ -115,7 +115,7 @@ class Github(HttpUpload):
def files_upload(self, release: dict[str, Any], local_files: dict[Path, str], remote_files: dict[str, str]) -> None:
"""
upload files to github
upload files to GitHub
Args:
release(dict[str, Any]): release object
@ -133,7 +133,7 @@ class Github(HttpUpload):
create empty release
Returns:
dict[str, Any]: github API release object for the new release
dict[str, Any]: GitHub API release object for the new release
"""
url = f"https://api.github.com/repos/{self.github_owner}/{self.github_repository}/releases"
response = self.make_request("POST", url, json={"tag_name": self.architecture, "name": self.architecture})
@ -145,7 +145,7 @@ class Github(HttpUpload):
get release object if any
Returns:
dict[str, Any] | None: github API release object if release found and None otherwise
dict[str, Any] | None: GitHub API release object if release found and None otherwise
"""
url = f"https://api.github.com/repos/{self.github_owner}/{self.github_repository}/releases/tags/{self.architecture}"
try:

View File

@ -80,19 +80,20 @@ class Upload(LazyLogging):
Upload: client according to current settings
"""
section, provider_name = configuration.gettype(target, architecture)
provider = UploadSettings.from_option(provider_name)
if provider == UploadSettings.Rsync:
match UploadSettings.from_option(provider_name):
case UploadSettings.Rsync:
from ahriman.core.upload.rsync import Rsync
return Rsync(architecture, configuration, section)
if provider == UploadSettings.S3:
case UploadSettings.S3:
from ahriman.core.upload.s3 import S3
return S3(architecture, configuration, section)
if provider == UploadSettings.Github:
case UploadSettings.GitHub:
from ahriman.core.upload.github import Github
return Github(architecture, configuration, section)
if provider == UploadSettings.RemoteService:
case UploadSettings.RemoteService:
from ahriman.core.upload.remote_service import RemoteService
return RemoteService(architecture, configuration, section)
case _:
return Upload(architecture, configuration) # should never happen
def run(self, path: Path, built_packages: list[Package]) -> None:

View File

@ -99,6 +99,11 @@ class UploadTrigger(Trigger):
"type": "string",
"allowed": ["ahriman", "remote-service"],
},
"timeout": {
"type": "integer",
"coerce": "integer",
"min": 0,
},
},
},
"s3": {

View File

@ -334,7 +334,7 @@ def pretty_size(size: float | None, level: int = 0) -> str:
Args:
size(float | None): size to convert
level(int, optional): represents current units, 0 is B, 1 is KiB, etc (Default value = 0)
level(int, optional): represents current units, 0 is B, 1 is KiB, etc. (Default value = 0)
Returns:
str: pretty printable size as string
@ -343,14 +343,16 @@ def pretty_size(size: float | None, level: int = 0) -> str:
OptionError: if size is more than 1TiB
"""
def str_level() -> str:
if level == 0:
match level:
case 0:
return "B"
if level == 1:
case 1:
return "KiB"
if level == 2:
case 2:
return "MiB"
if level == 3:
case 3:
return "GiB"
case _:
raise OptionError(level) # must never happen actually
if size is None:
@ -389,7 +391,7 @@ def srcinfo_property(key: str, srcinfo: dict[str, Any], package_srcinfo: dict[st
returned
Args:
key(str): key to extract from srcinfo
key(str): key to extract
srcinfo(dict[str, Any]): root structure of SRCINFO
package_srcinfo(dict[str, Any]): package specific SRCINFO
default(Any, optional): the default value for the specified key (Default value = None)
@ -408,7 +410,7 @@ def srcinfo_property_list(key: str, srcinfo: dict[str, Any], package_srcinfo: di
append it at the end of result
Args:
key(str): key to extract from srcinfo
key(str): key to extract
srcinfo(dict[str, Any]): root structure of SRCINFO
package_srcinfo(dict[str, Any]): package specific SRCINFO
architecture(str | None, optional): package architecture if set (Default value = None)
@ -424,9 +426,9 @@ def srcinfo_property_list(key: str, srcinfo: dict[str, Any], package_srcinfo: di
def trim_package(package_name: str) -> str:
"""
remove version bound and description from package name. Pacman allows to specify version bound (=, <=, >= etc) for
packages in dependencies and also allows to specify description (via :); this function removes trailing parts and
return exact package name
remove version bound and description from package name. Pacman allows to specify version bound (=, <=, >= etc.) for
packages in dependencies and also allows to specify description (via ``:``); this function removes trailing parts
and return exact package name
Args:
package_name(str): source package name

View File

@ -44,9 +44,7 @@ class AuthSettings(str, Enum):
Returns:
bool: False in case if authorization is disabled and True otherwise
"""
if self == AuthSettings.Disabled:
return False
return True
return self != AuthSettings.Disabled
@staticmethod
def from_option(value: str) -> AuthSettings:
@ -59,8 +57,10 @@ class AuthSettings(str, Enum):
Returns:
AuthSettings: parsed value
"""
if value.lower() in ("configuration", "mapping"):
match value.lower():
case "configuration" | "mapping":
return AuthSettings.Configuration
if value.lower() in ("oauth", "oauth2"):
case "oauth" | "oauth2":
return AuthSettings.OAuth
case _:
return AuthSettings.Disabled

View File

@ -64,7 +64,7 @@ class RemoteSource:
@property
def pkgbuild_dir(self) -> Path | None:
"""
get path to directory with package sources (PKGBUILD etc)
get path to directory with package sources (PKGBUILD etc.)
Returns:
Path | None: path to directory with package sources based on settings if available

View File

@ -53,14 +53,16 @@ class ReportSettings(str, Enum):
Returns:
ReportSettings: parsed value
"""
if value.lower() in ("html",):
match value.lower():
case "html":
return ReportSettings.HTML
if value.lower() in ("email",):
case "email":
return ReportSettings.Email
if value.lower() in ("console",):
case "console":
return ReportSettings.Console
if value.lower() in ("telegram",):
case "telegram":
return ReportSettings.Telegram
if value.lower() in ("ahriman", "remote-call",):
case "ahriman" | "remote-call":
return ReportSettings.RemoteCall
case _:
return ReportSettings.Disabled

View File

@ -32,8 +32,8 @@ class RepositoryPaths:
repository paths holder. For the most operations with paths you want to use this object
Attributes:
root(Path): repository root (i.e. ahriman home)
architecture(str): repository architecture
root(Path): repository root (i.e. ahriman home)
Examples:
This class can be used in order to access the repository tree structure::

View File

@ -47,8 +47,10 @@ class SignSettings(str, Enum):
Returns:
SignSettings: parsed value
"""
if value.lower() in ("package", "packages", "sign-package"):
match value.lower():
case "package" | "packages" | "sign-package":
return SignSettings.Packages
if value.lower() in ("repository", "sign-repository"):
case "repository" | "sign-repository":
return SignSettings.Repository
case _:
return SignSettings.Disabled

View File

@ -47,8 +47,10 @@ class SmtpSSLSettings(str, Enum):
Returns:
SmtpSSLSettings: parsed value
"""
if value.lower() in ("ssl", "ssl/tls"):
match value.lower():
case "ssl" | "ssl/tls":
return SmtpSSLSettings.SSL
if value.lower() in ("starttls",):
case "starttls":
return SmtpSSLSettings.STARTTLS
case _:
return SmtpSSLSettings.Disabled

View File

@ -30,14 +30,14 @@ class UploadSettings(str, Enum):
Disabled(UploadSettings): (class attribute) no sync will be performed, required for testing purpose
Rsync(UploadSettings): (class attribute) sync via rsync
S3(UploadSettings): (class attribute) sync to Amazon S3
Github(UploadSettings): (class attribute) sync to github releases page
GitHub(UploadSettings): (class attribute) sync to GitHub releases page
RemoteService(UploadSettings): (class attribute) sync to another ahriman instance
"""
Disabled = "disabled" # for testing purpose
Rsync = "rsync"
S3 = "s3"
Github = "github"
GitHub = "github"
RemoteService = "remote-service"
@staticmethod
@ -51,12 +51,14 @@ class UploadSettings(str, Enum):
Returns:
UploadSettings: parsed value
"""
if value.lower() in ("rsync",):
match value.lower():
case "rsync":
return UploadSettings.Rsync
if value.lower() in ("s3",):
case "s3":
return UploadSettings.S3
if value.lower() in ("github",):
return UploadSettings.Github
if value.lower() in ("ahriman", "remote-service",):
case "github":
return UploadSettings.GitHub
case "ahriman" | "remote-service":
return UploadSettings.RemoteService
case _:
return UploadSettings.Disabled

View File

@ -102,6 +102,7 @@ def _auth_handler(allow_read_only: bool) -> MiddlewareType:
permission = UserAccess.Unauthorized if isinstance(handler_instance, StaticResource) else UserAccess.Full
else:
permission = UserAccess.Full
if permission == UserAccess.Unauthorized: # explicit if elif else for better code coverage
pass
elif allow_read_only and UserAccess.Read.permits(permission):

View File

@ -107,7 +107,7 @@ class BaseView(View, CorsViewMixin):
get non-empty value from request parameters
Args:
extractor(Callable[[str], T | None]): function to get value by the specified key
extractor(Callable[[str], T | None]): function to get value
key(str): key to extract value
Returns:

View File

@ -28,7 +28,7 @@ def test_run(args: argparse.Namespace, configuration: Configuration, repository:
"""
must run command
"""
package_ahriman.version = "0.0.0"
package_ahriman.version = "0.0.0-1"
args = _default_args(args)
mocker.patch("ahriman.core.repository.Repository.load", return_value=repository)
package_mock = mocker.patch("ahriman.models.package.Package.from_aur", return_value=package_ahriman)

View File

@ -18,8 +18,8 @@ def test_from_option_valid() -> None:
assert UploadSettings.from_option("s3") == UploadSettings.S3
assert UploadSettings.from_option("S3") == UploadSettings.S3
assert UploadSettings.from_option("github") == UploadSettings.Github
assert UploadSettings.from_option("GitHub") == UploadSettings.Github
assert UploadSettings.from_option("github") == UploadSettings.GitHub
assert UploadSettings.from_option("GitHub") == UploadSettings.GitHub
assert UploadSettings.from_option("remote-service") == UploadSettings.RemoteService
assert UploadSettings.from_option("Remote-Service") == UploadSettings.RemoteService