From 5dfd4cf998807d30b2b9b022395f870512d6fbe9 Mon Sep 17 00:00:00 2001 From: Red-GitHubBot <88117545+Red-GitHubBot@users.noreply.github.com> Date: Sat, 25 Dec 2021 21:13:30 +0100 Subject: [PATCH] [3.4] Fix RPC cog load and unload by returning dicts (#5453) (#5490) * use dicts for CoreLogic package management returns * address review * failed_packages->notloaded_packages in _unload (cherry picked from commit d27dbded8a0b67d04014d956b371bcb675d5fcf8) Co-authored-by: Vexed Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> Co-authored-by: Vexed Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> --- redbot/core/core_commands.py | 149 +++++++++++++---------------------- 1 file changed, 56 insertions(+), 93 deletions(-) diff --git a/redbot/core/core_commands.py b/redbot/core/core_commands.py index aed620f0d..cdcc33601 100644 --- a/redbot/core/core_commands.py +++ b/redbot/core/core_commands.py @@ -114,11 +114,7 @@ class CoreLogic: self.bot.register_rpc_handler(self._version_info) self.bot.register_rpc_handler(self._invite_url) - async def _load( - self, pkg_names: Iterable[str] - ) -> Tuple[ - List[str], List[str], List[str], List[str], List[str], List[Tuple[str, str]], Set[str] - ]: + async def _load(self, pkg_names: Iterable[str]) -> Dict[str, Union[List[str], Dict[str, str]]]: """ Loads packages by name. @@ -129,23 +125,23 @@ class CoreLogic: Returns ------- - tuple - 7-tuple of: - 1. List of names of packages that loaded successfully - 2. List of names of packages that failed to load without specified reason - 3. List of names of packages that don't have a valid package name - 4. List of names of packages that weren't found in any cog path - 5. List of names of packages that are already loaded - 6. List of 2-tuples (pkg_name, reason) for packages - that failed to load with a specified reason - 7. Set of repo names that use deprecated shared libraries + dict + Dictionary with keys: + ``loaded_packages``: List of names of packages that loaded successfully + ``failed_packages``: List of names of packages that failed to load without specified reason + ``invalid_pkg_names``: List of names of packages that don't have a valid package name + ``notfound_packages``: List of names of packages that weren't found in any cog path + ``alreadyloaded_packages``: List of names of packages that are already loaded + ``failed_with_reason_packages``: Dictionary of packages that failed to load with + a specified reason with mapping of package names -> failure reason + ``repos_with_shared_libs``: List of repo names that use deprecated shared libraries """ failed_packages = [] loaded_packages = [] invalid_pkg_names = [] notfound_packages = [] alreadyloaded_packages = [] - failed_with_reason_packages = [] + failed_with_reason_packages = {} repos_with_shared_libs = set() bot = self.bot @@ -177,7 +173,7 @@ class CoreLogic: except errors.PackageAlreadyLoaded: alreadyloaded_packages.append(name) except errors.CogLoadError as e: - failed_with_reason_packages.append((name, str(e))) + failed_with_reason_packages[name] = str(e) except Exception as e: if isinstance(e, commands.CommandRegistrationError): if e.alias_conflict: @@ -190,7 +186,7 @@ class CoreLogic: "Command {command_name} is already an existing command" " or alias in one of the loaded cogs." ).format(command_name=inline(e.name)) - failed_with_reason_packages.append((name, error_message)) + failed_with_reason_packages[name] = error_message continue log.exception("Package loading failed", exc_info=e) @@ -217,15 +213,15 @@ class CoreLogic: if maybe_repo is not None: repos_with_shared_libs.add(maybe_repo.name) - return ( - loaded_packages, - failed_packages, - invalid_pkg_names, - notfound_packages, - alreadyloaded_packages, - failed_with_reason_packages, - repos_with_shared_libs, - ) + return { + "loaded_packages": loaded_packages, + "failed_packages": failed_packages, + "invalid_pkg_names": invalid_pkg_names, + "notfound_packages": notfound_packages, + "alreadyloaded_packages": alreadyloaded_packages, + "failed_with_reason_packages": failed_with_reason_packages, + "repos_with_shared_libs": list(repos_with_shared_libs), + } @staticmethod def _cleanup_and_refresh_modules(module_name: str) -> None: @@ -253,7 +249,7 @@ class CoreLogic: for child_name, lib in children.items(): importlib._bootstrap._exec(lib.__spec__, lib) - async def _unload(self, pkg_names: Iterable[str]) -> Tuple[List[str], List[str]]: + async def _unload(self, pkg_names: Iterable[str]) -> Dict[str, List[str]]: """ Unloads packages with the given names. @@ -264,10 +260,13 @@ class CoreLogic: Returns ------- - tuple - 2 element tuple of successful unloads and failed unloads. + dict + Dictionary with keys: + ``unloaded_packages``: List of names of packages that unloaded successfully. + ``notloaded_packages``: List of names of packages that weren't unloaded + because they weren't loaded. """ - failed_packages = [] + notloaded_packages = [] unloaded_packages = [] bot = self.bot @@ -278,15 +277,13 @@ class CoreLogic: await bot.remove_loaded_package(name) unloaded_packages.append(name) else: - failed_packages.append(name) + notloaded_packages.append(name) - return unloaded_packages, failed_packages + return {"unloaded_packages": unloaded_packages, "notloaded_packages": notloaded_packages} async def _reload( self, pkg_names: Sequence[str] - ) -> Tuple[ - List[str], List[str], List[str], List[str], List[str], List[Tuple[str, str]], Set[str] - ]: + ) -> Dict[str, Union[List[str], Dict[str, str]]]: """ Reloads packages with the given names. @@ -297,30 +294,12 @@ class CoreLogic: Returns ------- - tuple - Tuple as returned by `CoreLogic._load()` + dict + Dictionary with keys as returned by `CoreLogic._load()` """ await self._unload(pkg_names) - ( - loaded, - load_failed, - invalid_pkg_names, - not_found, - already_loaded, - load_failed_with_reason, - repos_with_shared_libs, - ) = await self._load(pkg_names) - - return ( - loaded, - load_failed, - invalid_pkg_names, - not_found, - already_loaded, - load_failed_with_reason, - repos_with_shared_libs, - ) + return await self._load(pkg_names) async def _name(self, name: Optional[str] = None) -> str: """ @@ -1699,24 +1678,16 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): """ cogs = tuple(map(lambda cog: cog.rstrip(","), cogs)) async with ctx.typing(): - ( - loaded, - failed, - invalid_pkg_names, - not_found, - already_loaded, - failed_with_reason, - repos_with_shared_libs, - ) = await self._load(cogs) + outcomes = await self._load(cogs) output = [] - if loaded: + if loaded := outcomes["loaded_packages"]: loaded_packages = humanize_list([inline(package) for package in loaded]) formed = _("Loaded {packs}.").format(packs=loaded_packages) output.append(formed) - if already_loaded: + if already_loaded := outcomes["alreadyloaded_packages"]: if len(already_loaded) == 1: formed = _("The following package is already loaded: {pack}").format( pack=inline(already_loaded[0]) @@ -1727,7 +1698,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ) output.append(formed) - if failed: + if failed := outcomes["failed_packages"]: if len(failed) == 1: formed = _( "Failed to load the following package: {pack}." @@ -1740,7 +1711,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(packs=humanize_list([inline(package) for package in failed])) output.append(formed) - if invalid_pkg_names: + if invalid_pkg_names := outcomes["invalid_pkg_names"]: if len(invalid_pkg_names) == 1: formed = _( "The following name is not a valid package name: {pack}\n" @@ -1755,7 +1726,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(packs=humanize_list([inline(package) for package in invalid_pkg_names])) output.append(formed) - if not_found: + if not_found := outcomes["notfound_packages"]: if len(not_found) == 1: formed = _("The following package was not found in any cog path: {pack}.").format( pack=inline(not_found[0]) @@ -1766,8 +1737,8 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(packs=humanize_list([inline(package) for package in not_found])) output.append(formed) - if failed_with_reason: - reasons = "\n".join([f"`{x}`: {y}" for x, y in failed_with_reason]) + if failed_with_reason := outcomes["failed_with_reason_packages"]: + reasons = "\n".join([f"`{x}`: {y}" for x, y in failed_with_reason.items()]) if len(failed_with_reason) == 1: formed = _( "This package could not be loaded for the following reason:\n\n{reason}" @@ -1778,7 +1749,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(reasons=reasons) output.append(formed) - if repos_with_shared_libs: + if repos_with_shared_libs := outcomes["repos_with_shared_libs"]: if len(repos_with_shared_libs) == 1: formed = _( "**WARNING**: The following repo is using shared libs" @@ -1817,11 +1788,11 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): - `` - The cog packages to unload. """ cogs = tuple(map(lambda cog: cog.rstrip(","), cogs)) - unloaded, failed = await self._unload(cogs) + outcomes = await self._unload(cogs) output = [] - if unloaded: + if unloaded := outcomes["unloaded_packages"]: if len(unloaded) == 1: formed = _("The following package was unloaded: {pack}.").format( pack=inline(unloaded[0]) @@ -1832,7 +1803,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ) output.append(formed) - if failed: + if failed := outcomes["notloaded_packages"]: if len(failed) == 1: formed = _("The following package was not loaded: {pack}.").format( pack=inline(failed[0]) @@ -1866,24 +1837,16 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): """ cogs = tuple(map(lambda cog: cog.rstrip(","), cogs)) async with ctx.typing(): - ( - loaded, - failed, - invalid_pkg_names, - not_found, - already_loaded, - failed_with_reason, - repos_with_shared_libs, - ) = await self._reload(cogs) + outcomes = await self._reload(cogs) output = [] - if loaded: + if loaded := outcomes["loaded_packages"]: loaded_packages = humanize_list([inline(package) for package in loaded]) formed = _("Reloaded {packs}.").format(packs=loaded_packages) output.append(formed) - if failed: + if failed := outcomes["failed_packages"]: if len(failed) == 1: formed = _( "Failed to reload the following package: {pack}." @@ -1896,7 +1859,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(packs=humanize_list([inline(package) for package in failed])) output.append(formed) - if invalid_pkg_names: + if invalid_pkg_names := outcomes["invalid_pkg_names"]: if len(invalid_pkg_names) == 1: formed = _( "The following name is not a valid package name: {pack}\n" @@ -1911,7 +1874,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(packs=humanize_list([inline(package) for package in invalid_pkg_names])) output.append(formed) - if not_found: + if not_found := outcomes["notfound_packages"]: if len(not_found) == 1: formed = _("The following package was not found in any cog path: {pack}.").format( pack=inline(not_found[0]) @@ -1922,8 +1885,8 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(packs=humanize_list([inline(package) for package in not_found])) output.append(formed) - if failed_with_reason: - reasons = "\n".join([f"`{x}`: {y}" for x, y in failed_with_reason]) + if failed_with_reason := outcomes["failed_with_reason_packages"]: + reasons = "\n".join([f"`{x}`: {y}" for x, y in failed_with_reason.items()]) if len(failed_with_reason) == 1: formed = _( "This package could not be reloaded for the following reason:\n\n{reason}" @@ -1934,7 +1897,7 @@ class Core(commands.commands._RuleDropper, commands.Cog, CoreLogic): ).format(reasons=reasons) output.append(formed) - if repos_with_shared_libs: + if repos_with_shared_libs := outcomes["repos_with_shared_libs"]: if len(repos_with_shared_libs) == 1: formed = _( "**WARNING**: The following repo is using shared libs"