From 01aa8bb2ba6fefed259fa8ab7983c40c23f30b06 Mon Sep 17 00:00:00 2001 From: Oliver Date: Thu, 21 Nov 2024 07:38:26 +1100 Subject: [PATCH] Plugins installation improvements (#8503) * Append plugins dir to pythonpath * Error handling in plugin helpers * Install plugin into "plugins" directory * Use plugins dir when installing from plugins.txt * Implement removal of plugin from plugins dir * Remove the dist-info dirs too * Cleanup * Catch errors * Specify plugin location for CI * Remove plugins.txt support * Improve regex for plugin matching * Revert "Remove plugins.txt support" This reverts commit 02783503513673574255bd1a809ce17c6f0cee6c. * Remove PLUGIN_ON_STARTUP support * Better error catching for broken packages * Cleanup * Revert "Cleanup" This reverts commit a40c85d47d9446cf4181b9865d2fce6a63efba92. * Improved exception handling for plugin loading * More logging * Revert uninstall behaviour * Revert python path update * Improve check for plugins file * Revert check on startup * Better management of plugins file - Use file hash to determine if it should be reloaded * Fix docstring * Update unit tests * revert gh env * No cache * Update src/backend/InvenTree/plugin/installer.py Co-authored-by: Matthias Mair * Use hashlib.file_digest * Remove --no-cache-dir * Revert "Use hashlib.file_digest" This reverts commit bf84c8155e6036da0a529acba44386fed982ae8e. * Add note for future selves --------- Co-authored-by: Matthias Mair --- src/backend/InvenTree/InvenTree/settings.py | 3 +- src/backend/InvenTree/InvenTree/tests.py | 14 +- src/backend/InvenTree/plugin/apps.py | 2 - src/backend/InvenTree/plugin/helpers.py | 20 ++- src/backend/InvenTree/plugin/installer.py | 137 ++++++++++++-------- src/backend/InvenTree/plugin/models.py | 2 +- src/backend/InvenTree/plugin/plugin.py | 5 +- src/backend/InvenTree/plugin/registry.py | 31 +++-- 8 files changed, 121 insertions(+), 93 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/settings.py b/src/backend/InvenTree/InvenTree/settings.py index be1b57d9d7..1567caae14 100644 --- a/src/backend/InvenTree/InvenTree/settings.py +++ b/src/backend/InvenTree/InvenTree/settings.py @@ -217,7 +217,8 @@ PLUGIN_RETRY = get_setting( 'INVENTREE_PLUGIN_RETRY', 'PLUGIN_RETRY', 3, typecast=int ) # How often should plugin loading be tried? -PLUGIN_FILE_CHECKED = False # Was the plugin file checked? +# Hash of the plugin file (will be updated on each change) +PLUGIN_FILE_HASH = '' STATICFILES_DIRS = [] diff --git a/src/backend/InvenTree/InvenTree/tests.py b/src/backend/InvenTree/InvenTree/tests.py index bf9ad84477..9080b521ea 100644 --- a/src/backend/InvenTree/InvenTree/tests.py +++ b/src/backend/InvenTree/InvenTree/tests.py @@ -1184,18 +1184,8 @@ class TestSettings(InvenTreeTestCase): """Test if install of plugins on startup works.""" from plugin import registry - if not settings.DOCKER: - # Check an install run - response = registry.install_plugin_file() - self.assertEqual(response, 'first_run') - - # Set dynamic setting to True and rerun to launch install - InvenTreeSetting.set_setting('PLUGIN_ON_STARTUP', True, self.user) - registry.reload_plugins(full_reload=True) - - # Check that there was another run - response = registry.install_plugin_file() - self.assertEqual(response, True) + registry.reload_plugins(full_reload=True, collect=True) + self.assertGreater(len(settings.PLUGIN_FILE_HASH), 0) def test_helpers_cfg_file(self): """Test get_config_file.""" diff --git a/src/backend/InvenTree/plugin/apps.py b/src/backend/InvenTree/plugin/apps.py index 8c037ba114..3c47bc70b0 100644 --- a/src/backend/InvenTree/plugin/apps.py +++ b/src/backend/InvenTree/plugin/apps.py @@ -23,8 +23,6 @@ class PluginAppConfig(AppConfig): def ready(self): """The ready method is extended to initialize plugins.""" - # skip loading if we run in a background thread - if not isInMainThread() and not isInWorkerThread(): return diff --git a/src/backend/InvenTree/plugin/helpers.py b/src/backend/InvenTree/plugin/helpers.py index 9fa392a6d2..75170b56fd 100644 --- a/src/backend/InvenTree/plugin/helpers.py +++ b/src/backend/InvenTree/plugin/helpers.py @@ -177,7 +177,17 @@ def get_modules(pkg, path=None): elif type(path) is not list: path = [path] - for finder, name, _ in pkgutil.walk_packages(path): + packages = pkgutil.walk_packages(path) + + while True: + try: + finder, name, _ = next(packages) + except StopIteration: + break + except Exception as error: + log_error({pkg.__name__: str(error)}, 'discovery') + continue + try: if sys.version_info < (3, 12): module = finder.find_module(name).load_module(name) @@ -202,9 +212,13 @@ def get_modules(pkg, path=None): return [v for k, v in context.items()] -def get_classes(module): +def get_classes(module) -> list: """Get all classes in a given module.""" - return inspect.getmembers(module, inspect.isclass) + try: + return inspect.getmembers(module, inspect.isclass) + except Exception: + log_error({module.__name__: 'Could not get classes'}, 'discovery') + return [] def get_plugins(pkg, baseclass, path=None): diff --git a/src/backend/InvenTree/plugin/installer.py b/src/backend/InvenTree/plugin/installer.py index 4d706aaad2..7f96871520 100644 --- a/src/backend/InvenTree/plugin/installer.py +++ b/src/backend/InvenTree/plugin/installer.py @@ -19,12 +19,15 @@ logger = logging.getLogger('inventree') def pip_command(*args): """Build and run a pip command using using the current python executable. - returns: subprocess.check_output - throws: subprocess.CalledProcessError + Returns: The output of the pip command + + Raises: + subprocess.CalledProcessError: If the pip command fails """ python = sys.executable command = [python, '-m', 'pip'] + command.extend(args) command = [str(x) for x in command] @@ -63,39 +66,55 @@ def handle_pip_error(error, path: str) -> list: raise ValidationError(errors[0]) -def check_package_path(packagename: str): - """Determine the install path of a particular package. +def get_install_info(packagename: str) -> dict: + """Determine the install information for a particular package. - - If installed, return the installation path - - If not installed, return False + - Uses 'pip show' to determine the install location of a package. """ - logger.debug('check_package_path: %s', packagename) + logger.debug('get_install_info: %s', packagename) # Remove version information - for c in '<>=! ': + for c in '<>=!@ ': packagename = packagename.split(c)[0] + info = {} + try: result = pip_command('show', packagename) output = result.decode('utf-8').split('\n') for line in output: - # Check if line matches pattern "Location: ..." - match = re.match(r'^Location:\s+(.+)$', line.strip()) + parts = line.split(':') - if match: - return match.group(1) + if len(parts) >= 2: + key = str(parts[0].strip().lower().replace('-', '_')) + value = str(parts[1].strip()) + + info[key] = value except subprocess.CalledProcessError as error: - log_error('check_package_path') + log_error('get_install_info') output = error.output.decode('utf-8') + info['error'] = output logger.exception('Plugin lookup failed: %s', str(output)) - return False - # If we get here, the package is not installed - return False + return info + + +def plugins_file_hash(): + """Return the file hash for the plugins file.""" + import hashlib + + pf = settings.PLUGIN_FILE + + if not pf or not pf.exists(): + return None + + with pf.open('rb') as f: + # Note: Once we support 3.11 as a minimum, we can use hashlib.file_digest + return hashlib.sha256(f.read()).hexdigest() def install_plugins_file(): @@ -108,8 +127,10 @@ def install_plugins_file(): logger.warning('Plugin file %s does not exist', str(pf)) return + cmd = ['install', '--disable-pip-version-check', '-U', '-r', str(pf)] + try: - pip_command('install', '-r', str(pf)) + pip_command(*cmd) except subprocess.CalledProcessError as error: output = error.output.decode('utf-8') logger.exception('Plugin file installation failed: %s', str(output)) @@ -120,17 +141,25 @@ def install_plugins_file(): log_error('pip') return False - # Update static files + # Collect plugin static files plugin.staticfiles.collect_plugins_static_files() - plugin.staticfiles.clear_plugins_static_files() # At this point, the plugins file has been installed return True -def update_plugins_file(install_name, remove=False): +def update_plugins_file(install_name, full_package=None, version=None, remove=False): """Add a plugin to the plugins file.""" - logger.info('Adding plugin to plugins file: %s', install_name) + if remove: + logger.info('Removing plugin from plugins file: %s', install_name) + else: + logger.info('Adding plugin to plugins file: %s', install_name) + + # If a full package name is provided, use that instead + if full_package and full_package != install_name: + new_value = full_package + else: + new_value = f'{install_name}=={version}' if version else install_name pf = settings.PLUGIN_FILE @@ -140,7 +169,7 @@ def update_plugins_file(install_name, remove=False): def compare_line(line: str): """Check if a line in the file matches the installname.""" - return line.strip().split('==')[0] == install_name.split('==')[0] + return re.match(rf'^{install_name}[\s=@]', line.strip()) # First, read in existing plugin file try: @@ -166,13 +195,13 @@ def update_plugins_file(install_name, remove=False): found = True if not remove: # Replace line with new install name - output.append(install_name) + output.append(new_value) else: output.append(line) # Append plugin to file if not found and not remove: - output.append(install_name) + output.append(new_value) # Write file back to disk try: @@ -203,15 +232,8 @@ def install_plugin(url=None, packagename=None, user=None, version=None): logger.info('install_plugin: %s, %s', url, packagename) - # Check if we are running in a virtual environment - # For now, just log a warning - in_venv = sys.prefix != sys.base_prefix - - if not in_venv: - logger.warning('InvenTree is not running in a virtual environment') - # build up the command - install_name = ['install', '-U'] + install_name = ['install', '-U', '--disable-pip-version-check'] full_pkg = '' @@ -246,23 +268,25 @@ def install_plugin(url=None, packagename=None, user=None, version=None): ret['result'] = ret['success'] = _('Installed plugin successfully') ret['output'] = str(result, 'utf-8') - if packagename and (path := check_package_path(packagename)): - # Override result information - ret['result'] = _(f'Installed plugin into {path}') + if packagename and (info := get_install_info(packagename)): + if path := info.get('location'): + ret['result'] = _(f'Installed plugin into {path}') + ret['version'] = info.get('version') except subprocess.CalledProcessError as error: handle_pip_error(error, 'plugin_install') - # Save plugin to plugins file - update_plugins_file(full_pkg) + if version := ret.get('version'): + # Save plugin to plugins file + update_plugins_file(packagename, full_package=full_pkg, version=version) - # Reload the plugin registry, to discover the new plugin - from plugin.registry import registry + # Reload the plugin registry, to discover the new plugin + from plugin.registry import registry - registry.reload_plugins(full_reload=True, force_reload=True, collect=True) + registry.reload_plugins(full_reload=True, force_reload=True, collect=True) - # Update static files - plugin.staticfiles.collect_plugins_static_files() + # Update static files + plugin.staticfiles.collect_plugins_static_files() return ret @@ -303,23 +327,24 @@ def uninstall_plugin(cfg: plugin.models.PluginConfig, user=None, delete_config=T _('Plugin cannot be uninstalled as it is currently active') ) + if not cfg.is_installed(): + raise ValidationError(_('Plugin is not installed')) + validate_package_plugin(cfg, user) package_name = cfg.package_name - logger.info('Uninstalling plugin: %s', package_name) - cmd = ['uninstall', '-y', package_name] + pkg_info = get_install_info(package_name) - try: - result = pip_command(*cmd) - - ret = { - 'result': _('Uninstalled plugin successfully'), - 'success': True, - 'output': str(result, 'utf-8'), - } - - except subprocess.CalledProcessError as error: - handle_pip_error(error, 'plugin_uninstall') + if path := pkg_info.get('location'): + # Uninstall the plugin using pip + logger.info('Uninstalling plugin: %s from %s', package_name, path) + try: + pip_command('uninstall', '-y', package_name) + except subprocess.CalledProcessError as error: + handle_pip_error(error, 'plugin_uninstall') + else: + # No matching install target found + raise ValidationError(_('Plugin installation not found')) # Update the plugins file update_plugins_file(package_name, remove=True) @@ -334,4 +359,4 @@ def uninstall_plugin(cfg: plugin.models.PluginConfig, user=None, delete_config=T # Reload the plugin registry registry.reload_plugins(full_reload=True, force_reload=True, collect=True) - return ret + return {'result': _('Uninstalled plugin successfully'), 'success': True} diff --git a/src/backend/InvenTree/plugin/models.py b/src/backend/InvenTree/plugin/models.py index 58c08101fd..95263caf52 100644 --- a/src/backend/InvenTree/plugin/models.py +++ b/src/backend/InvenTree/plugin/models.py @@ -70,7 +70,7 @@ class PluginConfig(InvenTree.models.MetadataMixin, models.Model): """Nice name for printing.""" name = f'{self.name} - {self.key}' if not self.active: - name += '(not active)' + name += ' (not active)' return name # extra attributes from the registry diff --git a/src/backend/InvenTree/plugin/plugin.py b/src/backend/InvenTree/plugin/plugin.py index 7ed154cf76..95c7cd6711 100644 --- a/src/backend/InvenTree/plugin/plugin.py +++ b/src/backend/InvenTree/plugin/plugin.py @@ -239,9 +239,10 @@ class InvenTreePlugin(VersionMixin, MixinBase, MetaBase): """File that contains plugin definition.""" return Path(inspect.getfile(cls)) - def path(self) -> Path: + @classmethod + def path(cls) -> Path: """Path to plugins base folder.""" - return self.file().parent + return cls.file().parent def _get_value(self, meta_name: str, package_name: str) -> str: """Extract values from class meta or package info. diff --git a/src/backend/InvenTree/plugin/registry.py b/src/backend/InvenTree/plugin/registry.py index 68b77da66f..5f75002035 100644 --- a/src/backend/InvenTree/plugin/registry.py +++ b/src/backend/InvenTree/plugin/registry.py @@ -287,6 +287,7 @@ class PluginsRegistry: if collect: logger.info('Collecting plugins') + self.install_plugin_file() self.plugin_modules = self.collect_plugins() self.plugins_loaded = False @@ -365,31 +366,32 @@ class PluginsRegistry: collected_plugins = [] # Collect plugins from paths - for plugin in self.plugin_dirs(): - logger.debug("Loading plugins from directory '%s'", plugin) + for plugin_dir in self.plugin_dirs(): + logger.debug("Loading plugins from directory '%s'", plugin_dir) parent_path = None - parent_obj = Path(plugin) + parent_obj = Path(plugin_dir) # If a "path" is provided, some special handling is required - if parent_obj.name is not plugin and len(parent_obj.parts) > 1: + if parent_obj.name is not plugin_dir and len(parent_obj.parts) > 1: # Ensure PosixPath object is converted to a string, before passing to get_plugins parent_path = str(parent_obj.parent) - plugin = parent_obj.name + plugin_dir = parent_obj.name # Gather Modules if parent_path: # On python 3.12 use new loader method if sys.version_info < (3, 12): raw_module = _load_source( - plugin, str(parent_obj.joinpath('__init__.py')) + plugin_dir, str(parent_obj.joinpath('__init__.py')) ) else: raw_module = SourceFileLoader( - plugin, str(parent_obj.joinpath('__init__.py')) + plugin_dir, str(parent_obj.joinpath('__init__.py')) ).load_module() else: - raw_module = importlib.import_module(plugin) + raw_module = importlib.import_module(plugin_dir) + modules = get_plugins(raw_module, InvenTreePlugin, path=parent_path) for item in modules or []: @@ -429,16 +431,13 @@ class PluginsRegistry: def install_plugin_file(self): """Make sure all plugins are installed in the current environment.""" - if settings.PLUGIN_FILE_CHECKED: - logger.info('Plugin file was already checked') - return True + from plugin.installer import install_plugins_file, plugins_file_hash - from plugin.installer import install_plugins_file + file_hash = plugins_file_hash() - if install_plugins_file(): - settings.PLUGIN_FILE_CHECKED = True - return 'first_run' - return False + if file_hash != settings.PLUGIN_FILE_HASH: + install_plugins_file() + settings.PLUGIN_FILE_HASH = file_hash # endregion