From ac197e7f2571c8e66a300e4db3e0298a334ebbe3 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 22 Apr 2024 16:13:20 -0400 Subject: [PATCH] Refactor galaxy.files plugin + config handling. - Downstream I want a way to load plugins from outside the ConfiguredFileSources class. - Adds some more typing as I went in there and refactored things. --- lib/galaxy/files/__init__.py | 102 ++++--------------- lib/galaxy/files/plugins.py | 106 ++++++++++++++++++++ lib/galaxy/files/sources/__init__.py | 7 +- lib/galaxy/files/unittest_utils/__init__.py | 10 +- lib/galaxy_test/api/test_drs.py | 4 +- test/unit/data/test_sniff_file_sources.py | 4 +- test/unit/files/_util.py | 4 +- test/unit/files/test_posix.py | 22 ++-- test/unit/job_execution/test_job_io.py | 6 +- 9 files changed, 153 insertions(+), 112 deletions(-) create mode 100644 lib/galaxy/files/plugins.py diff --git a/lib/galaxy/files/__init__.py b/lib/galaxy/files/__init__.py index 3ddf2a35b1da..f58f158ee38c 100644 --- a/lib/galaxy/files/__init__.py +++ b/lib/galaxy/files/__init__.py @@ -16,9 +16,16 @@ FilesSourceProperties, PluginKind, ) -from galaxy.util import plugin_config -from galaxy.util.config_parsers import parse_allowlist_ips from galaxy.util.dictifiable import Dictifiable +from galaxy.util.plugin_config import ( + plugin_source_from_dict, + plugin_source_from_path, + PluginConfigSource, +) +from .plugins import ( + FileSourcePluginLoader, + FileSourcePluginsConfig, +) log = logging.getLogger(__name__) @@ -44,18 +51,18 @@ class ConfiguredFileSources: def __init__( self, - file_sources_config: "ConfiguredFileSourcesConfig", + file_sources_config: FileSourcePluginsConfig, conf_file=None, conf_dict=None, load_stock_plugins=False, ): self._file_sources_config = file_sources_config - self._plugin_classes = self._file_source_plugins_dict() + self._plugin_loader = FileSourcePluginLoader() file_sources: List[BaseFilesSource] = [] if conf_file is not None: file_sources = self._load_plugins_from_file(conf_file) elif conf_dict is not None: - plugin_source = plugin_config.plugin_source_from_dict(conf_dict) + plugin_source = plugin_source_from_dict(conf_dict) file_sources = self._parse_plugin_source(plugin_source) else: file_sources = [] @@ -81,7 +88,7 @@ def _ensure_loaded(plugin_type): _ensure_loaded("gxuserimport") if stock_file_source_conf_dict: - stock_plugin_source = plugin_config.plugin_source_from_dict(stock_file_source_conf_dict) + stock_plugin_source = plugin_source_from_dict(stock_file_source_conf_dict) # insert at beginning instead of append so FTP and library import appear # at the top of the list (presumably the most common options). Admins can insert # these explicitly for greater control. @@ -90,25 +97,12 @@ def _ensure_loaded(plugin_type): self._file_sources = file_sources self.custom_sources_configured = custom_sources_configured - def _load_plugins_from_file(self, conf_file): - plugin_source = plugin_config.plugin_source_from_path(conf_file) + def _load_plugins_from_file(self, conf_file: str): + plugin_source = plugin_source_from_path(conf_file) return self._parse_plugin_source(plugin_source) - def _file_source_plugins_dict(self): - import galaxy.files.sources - - return plugin_config.plugins_dict(galaxy.files.sources, "plugin_type") - - def _parse_plugin_source(self, plugin_source): - extra_kwds = { - "file_sources_config": self._file_sources_config, - } - return plugin_config.load_plugins( - self._plugin_classes, - plugin_source, - extra_kwds, - dict_to_list_key="id", - ) + def _parse_plugin_source(self, plugin_source: PluginConfigSource): + return self._plugin_loader.load_plugins(plugin_source, self._file_sources_config) def find_best_match(self, url: str) -> Optional[BaseFilesSource]: """Returns the best matching file source for handling a particular url. Each filesource scores its own @@ -204,7 +198,7 @@ def from_app_config(config): if not config_file or not os.path.exists(config_file): config_file = None config_dict = config.file_sources - file_sources_config = ConfiguredFileSourcesConfig.from_app_config(config) + file_sources_config = FileSourcePluginsConfig.from_app_config(config) return ConfiguredFileSources( file_sources_config, conf_file=config_file, conf_dict=config_dict, load_stock_plugins=True ) @@ -214,10 +208,10 @@ def from_dict(as_dict, load_stock_plugins=False): if as_dict is not None: sources_as_dict = as_dict["file_sources"] config_as_dict = as_dict["config"] - file_sources_config = ConfiguredFileSourcesConfig.from_dict(config_as_dict) + file_sources_config = FileSourcePluginsConfig.from_dict(config_as_dict) else: sources_as_dict = [] - file_sources_config = ConfiguredFileSourcesConfig() + file_sources_config = FileSourcePluginsConfig() return ConfiguredFileSources( file_sources_config, conf_dict=sources_as_dict, load_stock_plugins=load_stock_plugins ) @@ -227,61 +221,7 @@ class NullConfiguredFileSources(ConfiguredFileSources): def __init__( self, ): - super().__init__(ConfiguredFileSourcesConfig()) - - -class ConfiguredFileSourcesConfig: - def __init__( - self, - symlink_allowlist=None, - fetch_url_allowlist=None, - library_import_dir=None, - user_library_import_dir=None, - ftp_upload_dir=None, - ftp_upload_purge=True, - ): - symlink_allowlist = symlink_allowlist or [] - fetch_url_allowlist = fetch_url_allowlist or [] - self.symlink_allowlist = symlink_allowlist - self.fetch_url_allowlist = fetch_url_allowlist - self.library_import_dir = library_import_dir - self.user_library_import_dir = user_library_import_dir - self.ftp_upload_dir = ftp_upload_dir - self.ftp_upload_purge = ftp_upload_purge - - @staticmethod - def from_app_config(config): - # Formalize what we read in from config to create a more clear interface - # for this component. - kwds = {} - kwds["symlink_allowlist"] = config.user_library_import_symlink_allowlist - kwds["fetch_url_allowlist"] = [str(ip) for ip in config.fetch_url_allowlist_ips] - kwds["library_import_dir"] = config.library_import_dir - kwds["user_library_import_dir"] = config.user_library_import_dir - kwds["ftp_upload_dir"] = config.ftp_upload_dir - kwds["ftp_upload_purge"] = config.ftp_upload_purge - return ConfiguredFileSourcesConfig(**kwds) - - def to_dict(self): - return { - "symlink_allowlist": self.symlink_allowlist, - "fetch_url_allowlist": self.fetch_url_allowlist, - "library_import_dir": self.library_import_dir, - "user_library_import_dir": self.user_library_import_dir, - "ftp_upload_dir": self.ftp_upload_dir, - "ftp_upload_purge": self.ftp_upload_purge, - } - - @staticmethod - def from_dict(as_dict): - return ConfiguredFileSourcesConfig( - symlink_allowlist=as_dict["symlink_allowlist"], - fetch_url_allowlist=parse_allowlist_ips(as_dict["fetch_url_allowlist"]), - library_import_dir=as_dict["library_import_dir"], - user_library_import_dir=as_dict["user_library_import_dir"], - ftp_upload_dir=as_dict["ftp_upload_dir"], - ftp_upload_purge=as_dict["ftp_upload_purge"], - ) + super().__init__(FileSourcePluginsConfig()) class FileSourceDictifiable(Dictifiable): diff --git a/lib/galaxy/files/plugins.py b/lib/galaxy/files/plugins.py new file mode 100644 index 000000000000..27f5f2de96d2 --- /dev/null +++ b/lib/galaxy/files/plugins.py @@ -0,0 +1,106 @@ +from typing import ( + List, + Optional, + TYPE_CHECKING, +) + +from galaxy.util.config_parsers import parse_allowlist_ips +from galaxy.util.plugin_config import ( + load_plugins, + PluginConfigSource, + plugins_dict, +) + +if TYPE_CHECKING: + from galaxy.files.sources import BaseFilesSource + + +class FileSourcePluginsConfig: + symlink_allowlist: List[str] + fetch_url_allowlist: List[str] + library_import_dir: Optional[str] + user_library_import_dir: Optional[str] + ftp_upload_dir: Optional[str] + ftp_upload_purge: bool + + def __init__( + self, + symlink_allowlist=None, + fetch_url_allowlist=None, + library_import_dir=None, + user_library_import_dir=None, + ftp_upload_dir=None, + ftp_upload_purge=True, + ): + symlink_allowlist = symlink_allowlist or [] + fetch_url_allowlist = fetch_url_allowlist or [] + self.symlink_allowlist = symlink_allowlist + self.fetch_url_allowlist = fetch_url_allowlist + self.library_import_dir = library_import_dir + self.user_library_import_dir = user_library_import_dir + self.ftp_upload_dir = ftp_upload_dir + self.ftp_upload_purge = ftp_upload_purge + + @staticmethod + def from_app_config(config): + # Formalize what we read in from config to create a more clear interface + # for this component. + kwds = {} + kwds["symlink_allowlist"] = config.user_library_import_symlink_allowlist + kwds["fetch_url_allowlist"] = [str(ip) for ip in config.fetch_url_allowlist_ips] + kwds["library_import_dir"] = config.library_import_dir + kwds["user_library_import_dir"] = config.user_library_import_dir + kwds["ftp_upload_dir"] = config.ftp_upload_dir + kwds["ftp_upload_purge"] = config.ftp_upload_purge + return FileSourcePluginsConfig(**kwds) + + def to_dict(self): + return { + "symlink_allowlist": self.symlink_allowlist, + "fetch_url_allowlist": self.fetch_url_allowlist, + "library_import_dir": self.library_import_dir, + "user_library_import_dir": self.user_library_import_dir, + "ftp_upload_dir": self.ftp_upload_dir, + "ftp_upload_purge": self.ftp_upload_purge, + } + + @staticmethod + def from_dict(as_dict): + return FileSourcePluginsConfig( + symlink_allowlist=as_dict["symlink_allowlist"], + fetch_url_allowlist=parse_allowlist_ips(as_dict["fetch_url_allowlist"]), + library_import_dir=as_dict["library_import_dir"], + user_library_import_dir=as_dict["user_library_import_dir"], + ftp_upload_dir=as_dict["ftp_upload_dir"], + ftp_upload_purge=as_dict["ftp_upload_purge"], + ) + + +class FileSourcePluginLoader: + + def __init__(self): + self._plugin_classes = self._file_source_plugins_dict() + + def _file_source_plugins_dict(self): + import galaxy.files.sources + + return plugins_dict(galaxy.files.sources, "plugin_type") + + def load_plugins( + self, plugin_source: PluginConfigSource, file_source_plugin_config: FileSourcePluginsConfig + ) -> List["BaseFilesSource"]: + extra_kwds = { + "file_sources_config": file_source_plugin_config, + } + return load_plugins( + self._plugin_classes, + plugin_source, + extra_kwds, + dict_to_list_key="id", + ) + + +__all__ = ( + "FileSourcePluginLoader", + "FileSourcePluginsConfig", +) diff --git a/lib/galaxy/files/sources/__init__.py b/lib/galaxy/files/sources/__init__.py index 80745cd4d41a..c0a79e7abe67 100644 --- a/lib/galaxy/files/sources/__init__.py +++ b/lib/galaxy/files/sources/__init__.py @@ -11,7 +11,6 @@ ClassVar, Optional, Set, - TYPE_CHECKING, ) from typing_extensions import ( @@ -24,6 +23,7 @@ ConfigurationError, ItemAccessibilityException, ) +from galaxy.files.plugins import FileSourcePluginsConfig from galaxy.util.bool_expressions import ( BooleanExpressionEvaluator, TokenContainedEvaluator, @@ -33,9 +33,6 @@ DEFAULT_SCHEME = "gxfiles" DEFAULT_WRITABLE = False -if TYPE_CHECKING: - from galaxy.files import ConfiguredFileSourcesConfig - class PluginKind(str, Enum): """Enum to distinguish between different kinds or categories of plugins.""" @@ -78,7 +75,7 @@ class FilesSourceProperties(TypedDict): filesource specific properties. """ - file_sources_config: NotRequired["ConfiguredFileSourcesConfig"] + file_sources_config: NotRequired[FileSourcePluginsConfig] id: NotRequired[str] label: NotRequired[str] doc: NotRequired[Optional[str]] diff --git a/lib/galaxy/files/unittest_utils/__init__.py b/lib/galaxy/files/unittest_utils/__init__.py index c0cf69096e78..c8ebbc13a5b3 100644 --- a/lib/galaxy/files/unittest_utils/__init__.py +++ b/lib/galaxy/files/unittest_utils/__init__.py @@ -2,14 +2,12 @@ import tempfile from typing import Tuple -from galaxy.files import ( - ConfiguredFileSources, - ConfiguredFileSourcesConfig, -) +from galaxy.files import ConfiguredFileSources +from galaxy.files.plugins import FileSourcePluginsConfig class TestConfiguredFileSources(ConfiguredFileSources): - def __init__(self, file_sources_config: ConfiguredFileSourcesConfig, conf_dict: dict, test_root: str): + def __init__(self, file_sources_config: FileSourcePluginsConfig, conf_dict: dict, test_root: str): super().__init__(file_sources_config, conf_dict=conf_dict) self.test_root = test_root @@ -22,7 +20,7 @@ def __init__(self, root: str): "type": "posix", "root": root, } - file_sources_config = ConfiguredFileSourcesConfig({}) + file_sources_config = FileSourcePluginsConfig({}) super().__init__(file_sources_config, {"test1": plugin}, root) diff --git a/lib/galaxy_test/api/test_drs.py b/lib/galaxy_test/api/test_drs.py index b1c77c00e1e1..038a1c25c7e8 100644 --- a/lib/galaxy_test/api/test_drs.py +++ b/lib/galaxy_test/api/test_drs.py @@ -13,9 +13,9 @@ from galaxy.files import ( ConfiguredFileSources, - ConfiguredFileSourcesConfig, DictFileSourcesUserContext, ) +from galaxy.files.plugins import FileSourcePluginsConfig from galaxy.files.sources.util import ( fetch_drs_to_file, RetryOptions, @@ -33,7 +33,7 @@ def user_context_fixture(): - file_sources_config = ConfiguredFileSourcesConfig(fetch_url_allowlist=parse_allowlist_ips(["127.0.0.0/24"])) + file_sources_config = FileSourcePluginsConfig(fetch_url_allowlist=parse_allowlist_ips(["127.0.0.0/24"])) file_sources = ConfiguredFileSources(file_sources_config, load_stock_plugins=True) user_context = DictFileSourcesUserContext( preferences={ diff --git a/test/unit/data/test_sniff_file_sources.py b/test/unit/data/test_sniff_file_sources.py index 84b81ee963d3..2267995f763a 100644 --- a/test/unit/data/test_sniff_file_sources.py +++ b/test/unit/data/test_sniff_file_sources.py @@ -1,7 +1,7 @@ import os from galaxy.datatypes import sniff -from galaxy.files import ConfiguredFileSourcesConfig +from galaxy.files.plugins import FileSourcePluginsConfig from galaxy.files.unittest_utils import ( setup_root, TestConfiguredFileSources, @@ -30,7 +30,7 @@ def _download_and_check_file(file_sources): def _configured_file_sources() -> TestConfiguredFileSources: tmp, root = setup_root() - file_sources_config = ConfiguredFileSourcesConfig() + file_sources_config = FileSourcePluginsConfig() plugin = { "type": "posix", } diff --git a/test/unit/files/_util.py b/test/unit/files/_util.py index 866552146e95..5da87676b120 100644 --- a/test/unit/files/_util.py +++ b/test/unit/files/_util.py @@ -5,9 +5,9 @@ from galaxy.files import ( ConfiguredFileSources, - ConfiguredFileSourcesConfig, DictFileSourcesUserContext, ) +from galaxy.files.plugins import FileSourcePluginsConfig TEST_USERNAME = "alice" TEST_EMAIL = "alice@galaxyproject.org" @@ -126,7 +126,7 @@ def write_from(file_sources, uri, content, user_context=None): def configured_file_sources(conf_file): - file_sources_config = ConfiguredFileSourcesConfig() + file_sources_config = FileSourcePluginsConfig() return ConfiguredFileSources(file_sources_config, conf_file=conf_file) diff --git a/test/unit/files/test_posix.py b/test/unit/files/test_posix.py index 56c0b1699e1d..579b8ee1ca60 100644 --- a/test/unit/files/test_posix.py +++ b/test/unit/files/test_posix.py @@ -12,10 +12,8 @@ ItemAccessibilityException, RequestParameterInvalidException, ) -from galaxy.files import ( - ConfiguredFileSources, - ConfiguredFileSourcesConfig, -) +from galaxy.files import ConfiguredFileSources +from galaxy.files.plugins import FileSourcePluginsConfig from galaxy.files.unittest_utils import ( setup_root, TestConfiguredFileSources, @@ -154,7 +152,7 @@ def test_posix_per_user_serialized(): def test_user_ftp_explicit_config(): - file_sources_config = ConfiguredFileSourcesConfig( + file_sources_config = FileSourcePluginsConfig( ftp_upload_purge=False, ) plugin = { @@ -179,7 +177,7 @@ def test_user_ftp_explicit_config(): def test_user_ftp_implicit_config(): tmp, root = setup_root() - file_sources_config = ConfiguredFileSourcesConfig( + file_sources_config = FileSourcePluginsConfig( ftp_upload_dir=root, ftp_upload_purge=False, ) @@ -197,7 +195,7 @@ def test_user_ftp_implicit_config(): def test_user_ftp_respects_upload_purge_off(): tmp, root = setup_root() - file_sources_config = ConfiguredFileSourcesConfig( + file_sources_config = FileSourcePluginsConfig( ftp_upload_dir=root, ftp_upload_purge=True, ) @@ -210,7 +208,7 @@ def test_user_ftp_respects_upload_purge_off(): def test_user_ftp_respects_upload_purge_on_by_default(): tmp, root = setup_root() - file_sources_config = ConfiguredFileSourcesConfig( + file_sources_config = FileSourcePluginsConfig( ftp_upload_dir=root, ) file_sources = ConfiguredFileSources(file_sources_config, conf_dict=[], load_stock_plugins=True) @@ -222,7 +220,7 @@ def test_user_ftp_respects_upload_purge_on_by_default(): def test_import_dir_explicit_config(): tmp, root = setup_root() - file_sources_config = ConfiguredFileSourcesConfig( + file_sources_config = FileSourcePluginsConfig( library_import_dir=root, ) plugin = { @@ -236,7 +234,7 @@ def test_import_dir_explicit_config(): def test_import_dir_implicit_config(): tmp, root = setup_root() - file_sources_config = ConfiguredFileSourcesConfig( + file_sources_config = FileSourcePluginsConfig( library_import_dir=root, ) file_sources = ConfiguredFileSources(file_sources_config, conf_dict=[], load_stock_plugins=True) @@ -247,7 +245,7 @@ def test_import_dir_implicit_config(): def test_user_import_dir_implicit_config(): tmp, root = setup_root() - file_sources_config = ConfiguredFileSourcesConfig( + file_sources_config = FileSourcePluginsConfig( user_library_import_dir=root, ) file_sources = ConfiguredFileSources(file_sources_config, conf_dict=[], load_stock_plugins=True) @@ -475,7 +473,7 @@ def _configured_file_sources_with_root( config_kwd = {} if include_allowlist: config_kwd["symlink_allowlist"] = [tmp] - file_sources_config = ConfiguredFileSourcesConfig(**config_kwd) + file_sources_config = FileSourcePluginsConfig(**config_kwd) plugin: Dict[str, Any] = { "type": "posix", } diff --git a/test/unit/job_execution/test_job_io.py b/test/unit/job_execution/test_job_io.py index 0f9d9c00dab0..749b972c5879 100644 --- a/test/unit/job_execution/test_job_io.py +++ b/test/unit/job_execution/test_job_io.py @@ -4,9 +4,9 @@ from galaxy.files import ( ConfiguredFileSources, - ConfiguredFileSourcesConfig, DictFileSourcesUserContext, ) +from galaxy.files.plugins import FileSourcePluginsConfig from galaxy.job_execution.setup import JobIO from galaxy.model import Job from galaxy.model.base import transaction @@ -29,7 +29,9 @@ class FileSourcesMockApp(GalaxyDataTestApp): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.file_sources = ConfiguredFileSources(ConfiguredFileSourcesConfig.from_app_config(self.config)) + self.file_sources = ConfiguredFileSources( + FileSourcePluginsConfig.from_app_config(self.config), + ) @pytest.fixture