From 16ba548b30aa9d08aabee2406edae2737a9c7090 Mon Sep 17 00:00:00 2001 From: kagami Date: Tue, 21 May 2024 11:15:23 +0800 Subject: [PATCH 1/5] Improve env checks, only reinstall dependencies when dependency files change --- devchat/workflow/env_manager.py | 116 ++++++++++++++++++++++++++++---- devchat/workflow/path.py | 8 +++ devchat/workflow/workflow.py | 6 +- 3 files changed, 114 insertions(+), 16 deletions(-) diff --git a/devchat/workflow/env_manager.py b/devchat/workflow/env_manager.py index 36e5223d..452066ba 100644 --- a/devchat/workflow/env_manager.py +++ b/devchat/workflow/env_manager.py @@ -1,10 +1,13 @@ +import hashlib import os import subprocess import sys from typing import Dict, Optional +from devchat.utils import get_logger + from .envs import MAMBA_BIN_PATH -from .path import MAMBA_PY_ENVS, MAMBA_ROOT +from .path import ENV_CACHE_DIR, MAMBA_PY_ENVS, MAMBA_ROOT from .schema import ExternalPyConf from .user_setting import USER_SETTINGS @@ -15,6 +18,8 @@ CONDA_FORGE_TUNA = "https://mirrors.tuna.tsinghua.edu.cn/anaconda/cloud/conda-forge/" PYPI_TUNA = "https://pypi.tuna.tsinghua.edu.cn/simple" +logger = get_logger(__name__) + def _get_external_envs() -> Dict[str, ExternalPyConf]: """ @@ -54,6 +59,15 @@ def get_py_version(py: str) -> Optional[str]: out = proc.stdout.read().decode("utf-8") return out.split()[1] + @staticmethod + def _get_dep_hash(reqirements_file: str) -> str: + """ + Get the hash of the requirements file content. + """ + with open(reqirements_file, "r", encoding="utf-8") as f: + content = f.read() + return hashlib.md5(content.encode("utf-8")).hexdigest() + def install(self, env_name: str, requirements_file: str) -> bool: """ Install requirements into the python environment. @@ -82,48 +96,124 @@ def install(self, env_name: str, requirements_file: str) -> bool: ] env = os.environ.copy() env.pop("PYTHONPATH") + # with subprocess.Popen(cmd, stdout=None, stderr=None, env=env) as proc: with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env) as proc: proc.wait() if proc.returncode != 0: - print(f"Failed to install requirements: {requirements_file}", flush=True) + logger.warning(f"Failed to install requirements: {requirements_file}", flush=True) return False return True - def ensure(self, env_name: str, py_version: str) -> Optional[str]: + def should_reinstall(self, env_name: str, requirements_file: str) -> bool: + """ + Check if the requirements file has been changed. + """ + cache_file = os.path.join(ENV_CACHE_DIR, f"{env_name}") + if not os.path.exists(cache_file): + return True + + dep_hash = self._get_dep_hash(requirements_file) + with open(cache_file, "r", encoding="utf-8") as f: + cache_hash = f.read() + + return dep_hash != cache_hash + + def ensure( + self, env_name: str, py_version: str, reqirements_file: Optional[str] = None + ) -> Optional[str]: """ Ensure the python environment exists with the given name and version. + And install the requirements if provided. return the python executable path. """ py = self.get_py(env_name) - should_remove = False + + should_remove_old = False if py: # check the version of the python executable current_version = self.get_py_version(py) - if current_version == py_version: - return py + if current_version != py_version: + should_remove_old = True - should_remove = True + if reqirements_file and self.should_reinstall(env_name, reqirements_file): + should_remove_old = True - print("\n```Step\n# Setting up workflow environment", flush=True) - print(f"\nenv_name: {env_name}") - print(f"python: {py_version}", flush=True) + if not should_remove_old: + return py - if should_remove: + print("\n```Step\n# Setting up workflow environment\n", flush=True) + if should_remove_old: + print(f"- Dependencies of {env_name} have been changed.", flush=True) + print(f"- Removing the old {env_name}...", flush=True) self.remove(env_name) # create the environment + print(f"- Creating {env_name} with {py_version}...", flush=True) create_ok = self.create(env_name, py_version) - print("\n```", flush=True) - if not create_ok: + print(f"- Failed to create {env_name}", flush=True) + print("\n```", flush=True) + # TODO: handle the error, show a user-friendly message return None + + # install the requirements + if reqirements_file: + filename = os.path.basename(reqirements_file) + print(f"- Installing requirements from {filename}...", flush=True) + install_ok = self.install(env_name, reqirements_file) + if not install_ok: + print(f"- Failed to install requirements from {filename}", flush=True) + print("\n```", flush=True) + # TODO: handle the error, show a user-friendly message + return None + + # save the hash of the requirements file content + dep_hash = self._get_dep_hash(reqirements_file) + cache_file = os.path.join(ENV_CACHE_DIR, f"{env_name}") + with open(cache_file, "w", encoding="utf-8") as f: + f.write(dep_hash) + + print("\n```", flush=True) return self.get_py(env_name) + # def ensure(self, env_name: str, py_version: str) -> Optional[str]: + # """ + # Ensure the python environment exists with the given name and version. + + # return the python executable path. + # """ + # py = self.get_py(env_name) + # should_remove = False + + # if py: + # # check the version of the python executable + # current_version = self.get_py_version(py) + + # if current_version == py_version: + # return py + + # should_remove = True + + # print("\n```Step\n# Setting up workflow environment", flush=True) + # print(f"\nenv_name: {env_name}") + # print(f"python: {py_version}", flush=True) + + # if should_remove: + # self.remove(env_name) + + # # create the environment + # create_ok = self.create(env_name, py_version) + # print("\n```", flush=True) + + # if not create_ok: + # return None + # return self.get_py(env_name) + def create(self, env_name: str, py_version: str) -> bool: """ Create a new python environment using mamba. diff --git a/devchat/workflow/path.py b/devchat/workflow/path.py index 49dc6a0e..9343dc44 100644 --- a/devchat/workflow/path.py +++ b/devchat/workflow/path.py @@ -20,6 +20,14 @@ COMMAND_FILENAMES = ["command.yml", "command.yaml"] +# ------------------------------- +# workflow related cache data +# ------------------------------- +CACHE_DIR = os.path.join(WORKFLOWS_BASE, "cache") +ENV_CACHE_DIR = os.path.join(CACHE_DIR, "env_cache") +os.makedirs(ENV_CACHE_DIR, exist_ok=True) + + # ------------------------------- # config & settings files paths # ------------------------------- diff --git a/devchat/workflow/workflow.py b/devchat/workflow/workflow.py index d9f8925e..9af8ff59 100644 --- a/devchat/workflow/workflow.py +++ b/devchat/workflow/workflow.py @@ -143,10 +143,10 @@ def setup( # TODO: 只在插件(IDE)启动后workflow第一次使用时ensure环境和依赖？ # Create workflow python env manager = PyEnvManager() - workflow_py = manager.ensure(pyconf.env_name, pyconf.version) + workflow_py = manager.ensure(pyconf.env_name, pyconf.version, pyconf.dependencies) - r_file = pyconf.dependencies - _ = manager.install(pyconf.env_name, r_file) + # r_file = pyconf.dependencies + # _ = manager.install(pyconf.env_name, r_file) runtime_param = { # from user interaction From bc276f2dfa6f1038389dfc3de0d19c206387aa62 Mon Sep 17 00:00:00 2001 From: kagami Date: Tue, 21 May 2024 14:42:57 +0800 Subject: [PATCH 2/5] Improve handling process output --- devchat/workflow/env_manager.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/devchat/workflow/env_manager.py b/devchat/workflow/env_manager.py index 452066ba..8d68798a 100644 --- a/devchat/workflow/env_manager.py +++ b/devchat/workflow/env_manager.py @@ -48,9 +48,7 @@ def get_py_version(py: str) -> Optional[str]: Get the version of the python executable. """ py_version_cmd = [py, "--version"] - with subprocess.Popen( - py_version_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE - ) as proc: + with subprocess.Popen(py_version_cmd, stdout=subprocess.PIPE, stderr=None) as proc: proc.wait() if proc.returncode != 0: @@ -93,15 +91,15 @@ def install(self, env_name: str, requirements_file: str) -> bool: requirements_file, "-i", PYPI_TUNA, + "--no-warn-script-location", ] env = os.environ.copy() env.pop("PYTHONPATH") - # with subprocess.Popen(cmd, stdout=None, stderr=None, env=env) as proc: - with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env) as proc: + with subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=None, env=env) as proc: proc.wait() if proc.returncode != 0: - logger.warning(f"Failed to install requirements: {requirements_file}", flush=True) + logger.warning(f"Failed to install requirements: {requirements_file}") return False return True @@ -164,10 +162,10 @@ def ensure( # install the requirements if reqirements_file: filename = os.path.basename(reqirements_file) - print(f"- Installing requirements from {filename}...", flush=True) + print(f"- Installing dependencies from {filename}...", flush=True) install_ok = self.install(env_name, reqirements_file) if not install_ok: - print(f"- Failed to install requirements from {filename}", flush=True) + print(f"- Failed to install dependencies from {filename}", flush=True) print("\n```", flush=True) # TODO: handle the error, show a user-friendly message return None @@ -235,7 +233,7 @@ def create(self, env_name: str, py_version: str) -> bool: f"python={py_version}", "-y", ] - with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as proc: + with subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=None) as proc: proc.wait() if proc.returncode != 0: @@ -262,7 +260,7 @@ def remove(self, env_name: str) -> bool: self.mamba_root, "-y", ] - with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as proc: + with subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=None) as proc: proc.wait() if proc.returncode != 0: From ede68c573a7eae37c35bbbc66ef9d585741882a1 Mon Sep 17 00:00:00 2001 From: kagami Date: Tue, 21 May 2024 16:18:49 +0800 Subject: [PATCH 3/5] Improve error handling and user messages --- devchat/workflow/env_manager.py | 49 +++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/devchat/workflow/env_manager.py b/devchat/workflow/env_manager.py index 8d68798a..56a3c833 100644 --- a/devchat/workflow/env_manager.py +++ b/devchat/workflow/env_manager.py @@ -2,7 +2,7 @@ import os import subprocess import sys -from typing import Dict, Optional +from typing import Dict, Optional, Tuple from devchat.utils import get_logger @@ -66,7 +66,7 @@ def _get_dep_hash(reqirements_file: str) -> str: content = f.read() return hashlib.md5(content.encode("utf-8")).hexdigest() - def install(self, env_name: str, requirements_file: str) -> bool: + def install(self, env_name: str, requirements_file: str) -> Tuple[bool, str]: """ Install requirements into the python environment. @@ -76,11 +76,11 @@ def install(self, env_name: str, requirements_file: str) -> bool: py = self.get_py(env_name) if not py: # TODO: raise error? - return False + return False, "Python executable not found." if not os.path.exists(requirements_file): # TODO: raise error? - return False + return False, "Dependencies file not found." cmd = [ py, @@ -95,14 +95,15 @@ def install(self, env_name: str, requirements_file: str) -> bool: ] env = os.environ.copy() env.pop("PYTHONPATH") - with subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=None, env=env) as proc: - proc.wait() + with subprocess.Popen( + cmd, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, env=env + ) as proc: + _, err = proc.communicate() if proc.returncode != 0: - logger.warning(f"Failed to install requirements: {requirements_file}") - return False + return False, err.decode("utf-8") - return True + return True, "" def should_reinstall(self, env_name: str, requirements_file: str) -> bool: """ @@ -152,22 +153,34 @@ def ensure( # create the environment print(f"- Creating {env_name} with {py_version}...", flush=True) - create_ok = self.create(env_name, py_version) + create_ok, msg = self.create(env_name, py_version) if not create_ok: print(f"- Failed to create {env_name}", flush=True) print("\n```", flush=True) - # TODO: handle the error, show a user-friendly message + print( + f"\n\nFailed to create {env_name}, the workflow will not run." + f"\n\nPlease try again later.", + flush=True, + ) + logger.warning(f"Failed to create {env_name}: {msg}") + sys.exit(0) return None # install the requirements if reqirements_file: filename = os.path.basename(reqirements_file) print(f"- Installing dependencies from {filename}...", flush=True) - install_ok = self.install(env_name, reqirements_file) + install_ok, msg = self.install(env_name, reqirements_file) if not install_ok: print(f"- Failed to install dependencies from {filename}", flush=True) print("\n```", flush=True) - # TODO: handle the error, show a user-friendly message + print( + "\n\nFailed to install dependencies, the workflow will not run." + "\n\nPlease try again later.", + flush=True, + ) + logger.warning(f"Failed to install dependencies: {msg}") + sys.exit(0) return None # save the hash of the requirements file content @@ -212,7 +225,7 @@ def ensure( # return None # return self.get_py(env_name) - def create(self, env_name: str, py_version: str) -> bool: + def create(self, env_name: str, py_version: str) -> Tuple[bool, str]: """ Create a new python environment using mamba. """ @@ -233,13 +246,13 @@ def create(self, env_name: str, py_version: str) -> bool: f"python={py_version}", "-y", ] - with subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=None) as proc: - proc.wait() + with subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) as proc: + _, err = proc.communicate() if proc.returncode != 0: - return False + return False, err.decode("utf-8") - return True + return True, "" def remove(self, env_name: str) -> bool: """ From 3f51b6ce9fcc51a7da8cc63441d3afea904d5797 Mon Sep 17 00:00:00 2001 From: kagami Date: Tue, 21 May 2024 16:24:35 +0800 Subject: [PATCH 4/5] Clean code --- devchat/workflow/env_manager.py | 129 ++++++++++++-------------------- devchat/workflow/workflow.py | 6 -- 2 files changed, 48 insertions(+), 87 deletions(-) diff --git a/devchat/workflow/env_manager.py b/devchat/workflow/env_manager.py index 56a3c833..56d461d6 100644 --- a/devchat/workflow/env_manager.py +++ b/devchat/workflow/env_manager.py @@ -58,67 +58,16 @@ def get_py_version(py: str) -> Optional[str]: return out.split()[1] @staticmethod - def _get_dep_hash(reqirements_file: str) -> str: + def get_dep_hash(reqirements_file: str) -> str: """ Get the hash of the requirements file content. + + Used to check if the requirements file has been changed. """ with open(reqirements_file, "r", encoding="utf-8") as f: content = f.read() return hashlib.md5(content.encode("utf-8")).hexdigest() - def install(self, env_name: str, requirements_file: str) -> Tuple[bool, str]: - """ - Install requirements into the python environment. - - Args: - requirements: the absolute path to the requirements file. - """ - py = self.get_py(env_name) - if not py: - # TODO: raise error? - return False, "Python executable not found." - - if not os.path.exists(requirements_file): - # TODO: raise error? - return False, "Dependencies file not found." - - cmd = [ - py, - "-m", - "pip", - "install", - "-r", - requirements_file, - "-i", - PYPI_TUNA, - "--no-warn-script-location", - ] - env = os.environ.copy() - env.pop("PYTHONPATH") - with subprocess.Popen( - cmd, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, env=env - ) as proc: - _, err = proc.communicate() - - if proc.returncode != 0: - return False, err.decode("utf-8") - - return True, "" - - def should_reinstall(self, env_name: str, requirements_file: str) -> bool: - """ - Check if the requirements file has been changed. - """ - cache_file = os.path.join(ENV_CACHE_DIR, f"{env_name}") - if not os.path.exists(cache_file): - return True - - dep_hash = self._get_dep_hash(requirements_file) - with open(cache_file, "r", encoding="utf-8") as f: - cache_hash = f.read() - - return dep_hash != cache_hash - def ensure( self, env_name: str, py_version: str, reqirements_file: Optional[str] = None ) -> Optional[str]: @@ -164,7 +113,7 @@ def ensure( ) logger.warning(f"Failed to create {env_name}: {msg}") sys.exit(0) - return None + # return None # install the requirements if reqirements_file: @@ -181,10 +130,10 @@ def ensure( ) logger.warning(f"Failed to install dependencies: {msg}") sys.exit(0) - return None + # return None # save the hash of the requirements file content - dep_hash = self._get_dep_hash(reqirements_file) + dep_hash = self.get_dep_hash(reqirements_file) cache_file = os.path.join(ENV_CACHE_DIR, f"{env_name}") with open(cache_file, "w", encoding="utf-8") as f: f.write(dep_hash) @@ -192,38 +141,56 @@ def ensure( print("\n```", flush=True) return self.get_py(env_name) - # def ensure(self, env_name: str, py_version: str) -> Optional[str]: - # """ - # Ensure the python environment exists with the given name and version. + def install(self, env_name: str, requirements_file: str) -> Tuple[bool, str]: + """ + Install requirements into the python environment. - # return the python executable path. - # """ - # py = self.get_py(env_name) - # should_remove = False + Args: + requirements: the absolute path to the requirements file. + """ + py = self.get_py(env_name) + if not py: + return False, "Python executable not found." - # if py: - # # check the version of the python executable - # current_version = self.get_py_version(py) + if not os.path.exists(requirements_file): + return False, "Dependencies file not found." - # if current_version == py_version: - # return py + cmd = [ + py, + "-m", + "pip", + "install", + "-r", + requirements_file, + "-i", + PYPI_TUNA, + "--no-warn-script-location", + ] + env = os.environ.copy() + env.pop("PYTHONPATH") + with subprocess.Popen( + cmd, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, env=env + ) as proc: + _, err = proc.communicate() - # should_remove = True + if proc.returncode != 0: + return False, err.decode("utf-8") - # print("\n```Step\n# Setting up workflow environment", flush=True) - # print(f"\nenv_name: {env_name}") - # print(f"python: {py_version}", flush=True) + return True, "" - # if should_remove: - # self.remove(env_name) + def should_reinstall(self, env_name: str, requirements_file: str) -> bool: + """ + Check if the requirements file has been changed. + """ + cache_file = os.path.join(ENV_CACHE_DIR, f"{env_name}") + if not os.path.exists(cache_file): + return True - # # create the environment - # create_ok = self.create(env_name, py_version) - # print("\n```", flush=True) + dep_hash = self.get_dep_hash(requirements_file) + with open(cache_file, "r", encoding="utf-8") as f: + cache_hash = f.read() - # if not create_ok: - # return None - # return self.get_py(env_name) + return dep_hash != cache_hash def create(self, env_name: str, py_version: str) -> Tuple[bool, str]: """ diff --git a/devchat/workflow/workflow.py b/devchat/workflow/workflow.py index 9af8ff59..40a1d95b 100644 --- a/devchat/workflow/workflow.py +++ b/devchat/workflow/workflow.py @@ -139,15 +139,9 @@ def setup( print("\n```", flush=True) else: - # TODO: 有没有更好的时机判断方法？既保证运行时一定安装了依赖、又不用每次都检查？ - # TODO: 只在插件(IDE)启动后workflow第一次使用时ensure环境和依赖？ - # Create workflow python env manager = PyEnvManager() workflow_py = manager.ensure(pyconf.env_name, pyconf.version, pyconf.dependencies) - # r_file = pyconf.dependencies - # _ = manager.install(pyconf.env_name, r_file) - runtime_param = { # from user interaction "model_name": model_name, From 8428005cfca565fd83bd3de6c51294f58c82084c Mon Sep 17 00:00:00 2001 From: kagami Date: Tue, 21 May 2024 17:03:58 +0800 Subject: [PATCH 5/5] Log more info when failed to create py env --- devchat/workflow/env_manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/devchat/workflow/env_manager.py b/devchat/workflow/env_manager.py index 56d461d6..41728bbc 100644 --- a/devchat/workflow/env_manager.py +++ b/devchat/workflow/env_manager.py @@ -213,12 +213,12 @@ def create(self, env_name: str, py_version: str) -> Tuple[bool, str]: f"python={py_version}", "-y", ] - with subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) as proc: - _, err = proc.communicate() + with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as proc: + out, err = proc.communicate() + msg = f"err: {err.decode()}\n-----\nout: {out.decode()}" if proc.returncode != 0: - return False, err.decode("utf-8") - + return False, msg return True, "" def remove(self, env_name: str) -> bool: