From 440d2252958c559d83f9b677232d4619f0d81621 Mon Sep 17 00:00:00 2001 From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com> Date: Mon, 15 Jul 2024 12:26:51 +0200 Subject: [PATCH 1/7] BROKEN - Test that second failed SSH task collection does not remove original folder --- tests/v2/00_ssh/test_task_collection_ssh.py | 27 ++++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/v2/00_ssh/test_task_collection_ssh.py b/tests/v2/00_ssh/test_task_collection_ssh.py index 514b138a7a..dcc9ff8c7b 100644 --- a/tests/v2/00_ssh/test_task_collection_ssh.py +++ b/tests/v2/00_ssh/test_task_collection_ssh.py @@ -31,33 +31,52 @@ async def test_task_collection_ssh( FRACTAL_SLURM_SSH_WORKING_BASE_DIR=remote_basedir, ) + # CASE 1: successful collection state = CollectionStateV2() db.add(state) await db.commit() await db.refresh(state) - task_pkg = _TaskCollectPip( package="fractal_tasks_core", package_version="1.0.2", python_version="3.9", ) - background_collect_pip_ssh( state_id=state.id, task_pkg=task_pkg, fractal_ssh=fractal_ssh, ) - await db.refresh(state) debug(state) assert state.data["status"] == "OK" - # Check that the remote folder exists (note: we can do it on the host # machine, because /tmp is shared with the container) venv_dir = Path(remote_basedir) / ".fractal/fractal-tasks-core1.0.2" debug(venv_dir) assert venv_dir.is_dir() + # CASE 2: Try collecting the same package again + state = CollectionStateV2() + db.add(state) + await db.commit() + await db.refresh(state) + background_collect_pip_ssh( + state_id=state.id, + task_pkg=task_pkg, + fractal_ssh=fractal_ssh, + ) + + await db.refresh(state) + debug(state) + # Check that the second collection failed, since folder already exists + assert state.data["status"] == "fail" + assert "already exists" in state.data["log"] + # Check that the remote folder was not removed (note: we can do it on the + # host machine, because /tmp is shared with the container) + venv_dir = Path(remote_basedir) / ".fractal/fractal-tasks-core1.0.2" + debug(venv_dir) + assert venv_dir.is_dir() + async def test_task_collection_ssh_failure( fractal_ssh: FractalSSH, From bc111c6d2ea422a1c4f0aa8868c86629dc4d9930 Mon Sep 17 00:00:00 2001 From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com> Date: Mon, 15 Jul 2024 12:53:00 +0200 Subject: [PATCH 2/7] Add `FabricSSH.rename_folder` method --- fractal_server/ssh/_fabric.py | 36 +++++++++++++++++++++++- tests/v2/00_ssh/test_FractalSSH.py | 45 ++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/fractal_server/ssh/_fabric.py b/fractal_server/ssh/_fabric.py index c888f21984..e30b91d41b 100644 --- a/fractal_server/ssh/_fabric.py +++ b/fractal_server/ssh/_fabric.py @@ -286,7 +286,6 @@ def remove_folder( folder: Absolute path to a folder that should be removed. safe_root: If `folder` is not a subfolder of the absolute `safe_root` path, raise an error. - fractal_ssh: """ invalid_characters = {" ", "\n", ";", "$", "`"} @@ -307,6 +306,41 @@ def remove_folder( cmd = f"rm -r {folder}" self.run_command(cmd=cmd) + def rename_folder( + self, + *, + source: str, + target: str, + ) -> None: + """ + Rename a folder remotely via SSH. + + This functions calls `mv`, after a few checks on `source` and + `target`. Note that `source` and `target` must be subfolders + of the same parent folder. + + Args: + source: + target: + """ + invalid_characters = {" ", "\n", ";", "$", "`"} + + if ( + not isinstance(source, str) + or not isinstance(target, str) + or len(invalid_characters.intersection(source)) > 0 + or len(invalid_characters.intersection(target)) > 0 + or not Path(source).is_absolute() + or not Path(target).is_absolute() + or Path(source).parent != Path(target).parent + ): + raise ValueError( + f"Invalid `rename_folder` arguments {source=} and {target=}." + ) + else: + cmd = f"mv {source} {target}" + self.run_command(cmd=cmd) + def get_ssh_connection( *, diff --git a/tests/v2/00_ssh/test_FractalSSH.py b/tests/v2/00_ssh/test_FractalSSH.py index 89b48f3061..f6cbab6cd8 100644 --- a/tests/v2/00_ssh/test_FractalSSH.py +++ b/tests/v2/00_ssh/test_FractalSSH.py @@ -169,7 +169,7 @@ def _send_file(remote: str, lock_timeout: float): def test_folder_utils(tmp777_path, fractal_ssh: FractalSSH): """ - Test basic working of `mkdir` and `remove_folder` methods. + Test basic working of `mkdir` and `rename_folder` methods. """ # Define folder @@ -193,8 +193,9 @@ def test_folder_utils(tmp777_path, fractal_ssh: FractalSSH): print(stdout) print() - # Remove folder - fractal_ssh.remove_folder(folder=folder, safe_root="/tmp") + # Rename folder + new_folder = (tmp777_path / "nested/new_folder").as_posix() + fractal_ssh.rename_folder(source=folder, target=new_folder) # Check that folder does not exist with pytest.raises(RuntimeError) as e: @@ -258,3 +259,41 @@ def test_remove_folder_input_validation(): safe_root="/actual_root", ) print(e.value) + + +def test_rename_folder_input_validation(): + """ + Test input validation of `remove_folder` method. + """ + fake_fractal_ssh = FractalSSH(connection=Connection(host="localhost")) + + # Folders which are just invalid + invalid_folders = [ + None, + " /somewhere", + "/ somewhere", + "somewhere", + "$(pwd)", + "`pwd`", + ] + for folder in invalid_folders: + with pytest.raises(ValueError) as e: + fake_fractal_ssh.rename_folder(source=folder, target="/something") + print(e.value) + with pytest.raises(ValueError) as e: + fake_fractal_ssh.rename_folder(source="/something", target=folder) + print(e.value) + + # Folders which are not in the same parent directory + with pytest.raises(ValueError) as e: + fake_fractal_ssh.rename_folder( + source="/something/a", target="/something/else/b" + ) + print(e.value) + + with pytest.raises(ValueError) as e: + fake_fractal_ssh.rename_folder( + source="/actual_root/../something", + target="/actual_root", + ) + print(e.value) From f202e058410c1a43c37f4b08b87c9a06596a207d Mon Sep 17 00:00:00 2001 From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com> Date: Mon, 15 Jul 2024 13:04:06 +0200 Subject: [PATCH 3/7] Use temporary folder for SSH task collection and review script templates --- .../tasks/v2/background_operations_ssh.py | 42 +++++++++++++++---- .../tasks/v2/templates/_1_create_venv.sh | 24 +++++++---- .../tasks/v2/templates/_2_upgrade_pip.sh | 8 +--- .../tasks/v2/templates/_3_pip_install.sh | 8 +--- .../tasks/v2/templates/_4_pip_freeze.sh | 10 +---- .../tasks/v2/templates/_5_pip_show.sh | 8 ++-- tests/v2/00_ssh/test_task_collection_ssh.py | 3 +- 7 files changed, 61 insertions(+), 42 deletions(-) diff --git a/fractal_server/tasks/v2/background_operations_ssh.py b/fractal_server/tasks/v2/background_operations_ssh.py index d7e181f1be..e1742c11a7 100644 --- a/fractal_server/tasks/v2/background_operations_ssh.py +++ b/fractal_server/tasks/v2/background_operations_ssh.py @@ -162,15 +162,28 @@ def background_collect_pip_ssh( install_string = ( f"{install_string}=={task_pkg.package_version}" ) - package_env_dir = ( + + # NOTE: `package_env_dir_tmp` should be used for all on-disk + # operations, while `package_env_dir` should be used when + # setting metadata (and for a preliminary check that it does + # not already exists) + basedir = ( Path(settings.FRACTAL_SLURM_SSH_WORKING_BASE_DIR) / ".fractal" - / f"{task_pkg.package_name}{package_version}" + ) + package_env_dir = ( + basedir / f"{task_pkg.package_name}{package_version}" + ).as_posix() + package_env_dir_tmp = ( + basedir / f"{task_pkg.package_name}{package_version}_tmp" ).as_posix() + logger.debug(f"{package_env_dir_tmp=}") + logger.debug(f"{package_env_dir=}") replacements = [ ("__PACKAGE_NAME__", task_pkg.package_name), ("__PACKAGE_ENV_DIR__", package_env_dir), + ("__PACKAGE_ENV_DIR_TMP__", package_env_dir_tmp), ("__PACKAGE__", task_pkg.package), ("__PYTHON__", python_bin), ("__INSTALL_STRING__", install_string), @@ -292,8 +305,19 @@ def background_collect_pip_ssh( _insert_tasks(task_list=task_list, db=db) logger.debug("collecting - END") - # Finalize (write metadata to DB) + # Finalize (move folder, write metadata to DB) logger.debug("finalising - START") + + logger.info( + f"Move remote folder {package_env_dir_tmp=} " + f"to {package_env_dir=}" + ) + fractal_ssh.rename_folder( + source=package_env_dir_tmp, + target=package_env_dir, + ) + logger.info(f"Moved temporary folder into {package_env_dir=}") + collection_state = db.get(CollectionStateV2, state_id) collection_state.data["log"] = log_file_path.open("r").read() collection_state.data["freeze"] = stdout_pip_freeze @@ -313,15 +337,19 @@ def background_collect_pip_ssh( db=db, ) try: - logger.info(f"Now delete remote folder {package_env_dir}") + logger.info( + f"Now delete remote folder {package_env_dir_tmp}" + ) fractal_ssh.remove_folder( - folder=package_env_dir, + folder=package_env_dir_tmp, safe_root=settings.FRACTAL_SLURM_SSH_WORKING_BASE_DIR, ) - logger.info(f"Deleted remoted folder {package_env_dir}") + logger.info( + f"Deleted remoted folder {package_env_dir_tmp}" + ) except Exception as e: logger.error( - f"Deleting remote folder failed.\n" + f"Removing remote folder failed.\n" f"Original error:\n{str(e)}" ) return diff --git a/fractal_server/tasks/v2/templates/_1_create_venv.sh b/fractal_server/tasks/v2/templates/_1_create_venv.sh index 1eda1b158e..bd8cb57ab4 100644 --- a/fractal_server/tasks/v2/templates/_1_create_venv.sh +++ b/fractal_server/tasks/v2/templates/_1_create_venv.sh @@ -8,28 +8,34 @@ write_log(){ # Variables to be filled within fractal-server PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ +PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ PYTHON=__PYTHON__ TIME_START=$(date +%s) - -# Create main folder +# Check that non-temporary folder does not exist if [ -d "$PACKAGE_ENV_DIR" ]; then write_log "ERROR: Folder $PACKAGE_ENV_DIR already exists. Exit." exit 1 fi -write_log "START mkdir -p $PACKAGE_ENV_DIR" -mkdir -p $PACKAGE_ENV_DIR -write_log "END mkdir -p $PACKAGE_ENV_DIR" + +# Create temporary folder +if [ -d "$PACKAGE_ENV_DIR_TMP" ]; then + write_log "ERROR: Folder $PACKAGE_ENV_DIR_TMP already exists. Exit." + exit 1 +fi +write_log "START mkdir -p $PACKAGE_ENV_DIR_TMP" +mkdir -p $PACKAGE_ENV_DIR_TMP +write_log "END mkdir -p $PACKAGE_ENV_DIR_TMP" echo # Create venv -write_log "START create venv in ${PACKAGE_ENV_DIR}" -"$PYTHON" -m venv "$PACKAGE_ENV_DIR" --copies -write_log "END create venv in ${PACKAGE_ENV_DIR}" +write_log "START create venv in ${PACKAGE_ENV_DIR_TMP}" +"$PYTHON" -m venv "$PACKAGE_ENV_DIR_TMP" --copies +write_log "END create venv in ${PACKAGE_ENV_DIR_TMP}" echo -VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python if [ -f "$VENVPYTHON" ]; then write_log "OK: $VENVPYTHON exists." echo diff --git a/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh b/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh index 65d7921275..440c823ac7 100644 --- a/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh +++ b/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh @@ -6,15 +6,11 @@ write_log(){ } # Variables to be filled within fractal-server -PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ -PACKAGE_NAME=__PACKAGE_NAME__ -PACKAGE=__PACKAGE__ -PYTHON=__PYTHON__ -INSTALL_STRING=__INSTALL_STRING__ +PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ TIME_START=$(date +%s) -VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python # Upgrade pip write_log "START upgrade pip" diff --git a/fractal_server/tasks/v2/templates/_3_pip_install.sh b/fractal_server/tasks/v2/templates/_3_pip_install.sh index 5644ec5a7b..cc88804092 100644 --- a/fractal_server/tasks/v2/templates/_3_pip_install.sh +++ b/fractal_server/tasks/v2/templates/_3_pip_install.sh @@ -7,16 +7,12 @@ write_log(){ # Variables to be filled within fractal-server -PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ -PACKAGE_NAME=__PACKAGE_NAME__ -PACKAGE=__PACKAGE__ -PYTHON=__PYTHON__ +PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ INSTALL_STRING=__INSTALL_STRING__ - TIME_START=$(date +%s) -VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python # Install package write_log "START install ${INSTALL_STRING}" diff --git a/fractal_server/tasks/v2/templates/_4_pip_freeze.sh b/fractal_server/tasks/v2/templates/_4_pip_freeze.sh index f7ba0a860c..3c89b12c5b 100644 --- a/fractal_server/tasks/v2/templates/_4_pip_freeze.sh +++ b/fractal_server/tasks/v2/templates/_4_pip_freeze.sh @@ -8,14 +8,8 @@ write_log(){ # Variables to be filled within fractal-server -PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ -PACKAGE_NAME=__PACKAGE_NAME__ -PACKAGE=__PACKAGE__ -PYTHON=__PYTHON__ -INSTALL_STRING=__INSTALL_STRING__ +PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ - - -VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python "$VENVPYTHON" -m pip freeze diff --git a/fractal_server/tasks/v2/templates/_5_pip_show.sh b/fractal_server/tasks/v2/templates/_5_pip_show.sh index 977fbe496c..f4700c6984 100644 --- a/fractal_server/tasks/v2/templates/_5_pip_show.sh +++ b/fractal_server/tasks/v2/templates/_5_pip_show.sh @@ -7,16 +7,14 @@ write_log(){ # Variables to be filled within fractal-server -PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ +PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ PACKAGE_NAME=__PACKAGE_NAME__ -PACKAGE=__PACKAGE__ -PYTHON=__PYTHON__ -INSTALL_STRING=__INSTALL_STRING__ + TIME_START=$(date +%s) -VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python write_log "Python interpreter: $VENVPYTHON" echo diff --git a/tests/v2/00_ssh/test_task_collection_ssh.py b/tests/v2/00_ssh/test_task_collection_ssh.py index dcc9ff8c7b..457b1d23fc 100644 --- a/tests/v2/00_ssh/test_task_collection_ssh.py +++ b/tests/v2/00_ssh/test_task_collection_ssh.py @@ -49,6 +49,7 @@ async def test_task_collection_ssh( await db.refresh(state) debug(state) assert state.data["status"] == "OK" + # Check that the remote folder exists (note: we can do it on the host # machine, because /tmp is shared with the container) venv_dir = Path(remote_basedir) / ".fractal/fractal-tasks-core1.0.2" @@ -66,9 +67,9 @@ async def test_task_collection_ssh( fractal_ssh=fractal_ssh, ) + # Check that the second collection failed, since folder already exists await db.refresh(state) debug(state) - # Check that the second collection failed, since folder already exists assert state.data["status"] == "fail" assert "already exists" in state.data["log"] # Check that the remote folder was not removed (note: we can do it on the From bcc0aed1bffa29b978f19588efebed94a81a0a54 Mon Sep 17 00:00:00 2001 From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com> Date: Mon, 15 Jul 2024 13:10:22 +0200 Subject: [PATCH 4/7] Update CHANGELOG [skip ci] --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c4e252702..ebfb0972aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ **Note**: Numbers like (\#1234) point to closed Pull Requests on the fractal-server repository. +# 2.3.3 + +This release fixes a SSH-task-collection bug introduced in version 2.3.1. + +* SSH features: + * Fix wrong removal of task-package folder upon task-collection failure (\#1649). + # 2.3.2 > **WARNING**: The remove-remote-venv-folder in the SSH task collection is broken (see issue 1633). Do not deploy this version in an SSH-based `fractal-server` instance. From 2acff137fd90d5149f0ec1db9995a86f4bd9cf87 Mon Sep 17 00:00:00 2001 From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com> Date: Wed, 17 Jul 2024 08:47:35 +0200 Subject: [PATCH 5/7] Change strategy to determine whether venv folder must be removed upon failure --- .../tasks/v2/background_operations_ssh.py | 73 ++++++++----------- .../tasks/v2/templates/_1_create_venv.sh | 22 ++---- .../tasks/v2/templates/_2_upgrade_pip.sh | 4 +- .../tasks/v2/templates/_3_pip_install.sh | 4 +- .../tasks/v2/templates/_4_pip_freeze.sh | 4 +- .../tasks/v2/templates/_5_pip_show.sh | 4 +- 6 files changed, 48 insertions(+), 63 deletions(-) diff --git a/fractal_server/tasks/v2/background_operations_ssh.py b/fractal_server/tasks/v2/background_operations_ssh.py index e1742c11a7..ab0a4b967e 100644 --- a/fractal_server/tasks/v2/background_operations_ssh.py +++ b/fractal_server/tasks/v2/background_operations_ssh.py @@ -162,28 +162,15 @@ def background_collect_pip_ssh( install_string = ( f"{install_string}=={task_pkg.package_version}" ) - - # NOTE: `package_env_dir_tmp` should be used for all on-disk - # operations, while `package_env_dir` should be used when - # setting metadata (and for a preliminary check that it does - # not already exists) - basedir = ( + package_env_dir = ( Path(settings.FRACTAL_SLURM_SSH_WORKING_BASE_DIR) / ".fractal" - ) - package_env_dir = ( - basedir / f"{task_pkg.package_name}{package_version}" + / f"{task_pkg.package_name}{package_version}" ).as_posix() - package_env_dir_tmp = ( - basedir / f"{task_pkg.package_name}{package_version}_tmp" - ).as_posix() - logger.debug(f"{package_env_dir_tmp=}") logger.debug(f"{package_env_dir=}") - replacements = [ ("__PACKAGE_NAME__", task_pkg.package_name), ("__PACKAGE_ENV_DIR__", package_env_dir), - ("__PACKAGE_ENV_DIR_TMP__", package_env_dir_tmp), ("__PACKAGE__", task_pkg.package), ("__PYTHON__", python_bin), ("__INSTALL_STRING__", install_string), @@ -210,10 +197,18 @@ def background_collect_pip_ssh( # long operations that do not use the db db.close() + # `remove_venv_folder_upon_failure` is set to True only if + # script 1 goes through, which means that the remote folder + # `package_env_dir` did not already exist. If this remote + # folder already existed, then script 1 fails and the boolean + # flag `remove_venv_folder_upon_failure` remains false. + remove_venv_folder_upon_failure = False stdout = _customize_and_run_template( script_filename="_1_create_venv.sh", **common_args, ) + remove_venv_folder_upon_failure = True + stdout = _customize_and_run_template( script_filename="_2_upgrade_pip.sh", **common_args, @@ -308,16 +303,6 @@ def background_collect_pip_ssh( # Finalize (move folder, write metadata to DB) logger.debug("finalising - START") - logger.info( - f"Move remote folder {package_env_dir_tmp=} " - f"to {package_env_dir=}" - ) - fractal_ssh.rename_folder( - source=package_env_dir_tmp, - target=package_env_dir, - ) - logger.info(f"Moved temporary folder into {package_env_dir=}") - collection_state = db.get(CollectionStateV2, state_id) collection_state.data["log"] = log_file_path.open("r").read() collection_state.data["freeze"] = stdout_pip_freeze @@ -336,20 +321,26 @@ def background_collect_pip_ssh( exception=e, db=db, ) - try: - logger.info( - f"Now delete remote folder {package_env_dir_tmp}" - ) - fractal_ssh.remove_folder( - folder=package_env_dir_tmp, - safe_root=settings.FRACTAL_SLURM_SSH_WORKING_BASE_DIR, - ) - logger.info( - f"Deleted remoted folder {package_env_dir_tmp}" - ) - except Exception as e: - logger.error( - f"Removing remote folder failed.\n" - f"Original error:\n{str(e)}" - ) + if remove_venv_folder_upon_failure: + try: + logger.info( + f"Now delete remote folder {package_env_dir}" + ) + fractal_ssh.remove_folder( + folder=package_env_dir, + safe_root=settings.FRACTAL_SLURM_SSH_WORKING_BASE_DIR, # noqa: E501 + ) + logger.info( + f"Deleted remoted folder {package_env_dir}" + ) + except Exception as e: + logger.error( + f"Removing remote folder failed.\n" + f"Original error:\n{str(e)}" + ) + else: + logger.error( + "Not trying to remove remote folder " + f"{package_env_dir}." + ) return diff --git a/fractal_server/tasks/v2/templates/_1_create_venv.sh b/fractal_server/tasks/v2/templates/_1_create_venv.sh index bd8cb57ab4..e92d830da8 100644 --- a/fractal_server/tasks/v2/templates/_1_create_venv.sh +++ b/fractal_server/tasks/v2/templates/_1_create_venv.sh @@ -8,34 +8,28 @@ write_log(){ # Variables to be filled within fractal-server PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ -PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ PYTHON=__PYTHON__ TIME_START=$(date +%s) -# Check that non-temporary folder does not exist +# Check that package folder does not exist if [ -d "$PACKAGE_ENV_DIR" ]; then write_log "ERROR: Folder $PACKAGE_ENV_DIR already exists. Exit." exit 1 fi -# Create temporary folder -if [ -d "$PACKAGE_ENV_DIR_TMP" ]; then - write_log "ERROR: Folder $PACKAGE_ENV_DIR_TMP already exists. Exit." - exit 1 -fi -write_log "START mkdir -p $PACKAGE_ENV_DIR_TMP" -mkdir -p $PACKAGE_ENV_DIR_TMP -write_log "END mkdir -p $PACKAGE_ENV_DIR_TMP" +write_log "START mkdir -p $PACKAGE_ENV_DIR" +mkdir -p $PACKAGE_ENV_DIR +write_log "END mkdir -p $PACKAGE_ENV_DIR" echo # Create venv -write_log "START create venv in ${PACKAGE_ENV_DIR_TMP}" -"$PYTHON" -m venv "$PACKAGE_ENV_DIR_TMP" --copies -write_log "END create venv in ${PACKAGE_ENV_DIR_TMP}" +write_log "START create venv in ${PACKAGE_ENV_DIR}" +"$PYTHON" -m venv "$PACKAGE_ENV_DIR" --copies +write_log "END create venv in ${PACKAGE_ENV_DIR}" echo -VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python if [ -f "$VENVPYTHON" ]; then write_log "OK: $VENVPYTHON exists." echo diff --git a/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh b/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh index 440c823ac7..b37875cd10 100644 --- a/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh +++ b/fractal_server/tasks/v2/templates/_2_upgrade_pip.sh @@ -6,11 +6,11 @@ write_log(){ } # Variables to be filled within fractal-server -PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ +PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ TIME_START=$(date +%s) -VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python # Upgrade pip write_log "START upgrade pip" diff --git a/fractal_server/tasks/v2/templates/_3_pip_install.sh b/fractal_server/tasks/v2/templates/_3_pip_install.sh index cc88804092..b71471d2c8 100644 --- a/fractal_server/tasks/v2/templates/_3_pip_install.sh +++ b/fractal_server/tasks/v2/templates/_3_pip_install.sh @@ -7,12 +7,12 @@ write_log(){ # Variables to be filled within fractal-server -PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ +PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ INSTALL_STRING=__INSTALL_STRING__ TIME_START=$(date +%s) -VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python # Install package write_log "START install ${INSTALL_STRING}" diff --git a/fractal_server/tasks/v2/templates/_4_pip_freeze.sh b/fractal_server/tasks/v2/templates/_4_pip_freeze.sh index 3c89b12c5b..73cf14dc08 100644 --- a/fractal_server/tasks/v2/templates/_4_pip_freeze.sh +++ b/fractal_server/tasks/v2/templates/_4_pip_freeze.sh @@ -8,8 +8,8 @@ write_log(){ # Variables to be filled within fractal-server -PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ +PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ -VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python "$VENVPYTHON" -m pip freeze diff --git a/fractal_server/tasks/v2/templates/_5_pip_show.sh b/fractal_server/tasks/v2/templates/_5_pip_show.sh index f4700c6984..14bc96fcf8 100644 --- a/fractal_server/tasks/v2/templates/_5_pip_show.sh +++ b/fractal_server/tasks/v2/templates/_5_pip_show.sh @@ -7,14 +7,14 @@ write_log(){ # Variables to be filled within fractal-server -PACKAGE_ENV_DIR_TMP=__PACKAGE_ENV_DIR_TMP__ +PACKAGE_ENV_DIR=__PACKAGE_ENV_DIR__ PACKAGE_NAME=__PACKAGE_NAME__ TIME_START=$(date +%s) -VENVPYTHON=${PACKAGE_ENV_DIR_TMP}/bin/python +VENVPYTHON=${PACKAGE_ENV_DIR}/bin/python write_log "Python interpreter: $VENVPYTHON" echo From bb678689fbfeddad0c8f6d81960a092f4bf446bd Mon Sep 17 00:00:00 2001 From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com> Date: Wed, 17 Jul 2024 08:51:36 +0200 Subject: [PATCH 6/7] Fix comment [skip ci] --- fractal_server/tasks/v2/background_operations_ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fractal_server/tasks/v2/background_operations_ssh.py b/fractal_server/tasks/v2/background_operations_ssh.py index ab0a4b967e..a866463e0f 100644 --- a/fractal_server/tasks/v2/background_operations_ssh.py +++ b/fractal_server/tasks/v2/background_operations_ssh.py @@ -300,7 +300,7 @@ def background_collect_pip_ssh( _insert_tasks(task_list=task_list, db=db) logger.debug("collecting - END") - # Finalize (move folder, write metadata to DB) + # Finalize (write metadata to DB) logger.debug("finalising - START") collection_state = db.get(CollectionStateV2, state_id) From 735515fcf776ece3b3d4d16a09ffd5cc59506d68 Mon Sep 17 00:00:00 2001 From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com> Date: Wed, 17 Jul 2024 09:04:22 +0200 Subject: [PATCH 7/7] Fix logging level [skip ci] --- fractal_server/tasks/v2/background_operations_ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fractal_server/tasks/v2/background_operations_ssh.py b/fractal_server/tasks/v2/background_operations_ssh.py index a866463e0f..3423232dae 100644 --- a/fractal_server/tasks/v2/background_operations_ssh.py +++ b/fractal_server/tasks/v2/background_operations_ssh.py @@ -339,7 +339,7 @@ def background_collect_pip_ssh( f"Original error:\n{str(e)}" ) else: - logger.error( + logger.info( "Not trying to remove remote folder " f"{package_env_dir}." )