Merge pull request #170 from rtCamp/fix/migration-executor

Fix migration executor logic on migration failure in benches and exit out `fm update` when bench not running.

frappe_manager/commands.py

@@ -1,4 +1,5 @@
 from pathlib import Path
+from frappe_manager.site_manager.site_exceptions import BenchNotRunning
 from frappe_manager.utils.site import pull_docker_images
 import typer
 import os
@@ -499,6 +500,9 @@ def update(
     restart_required = False
     bench_config_save = False
 
+    if not bench.compose_project.running:
+        raise BenchNotRunning(bench_name=bench.name)
+
     if developer_mode:
         if developer_mode == EnableDisableOptionsEnum.enable:
             bench.bench_config.developer_mode = True

frappe_manager/compose_project/compose_project.py

@@ -3,6 +3,7 @@
 from rich.text import Text
 from frappe_manager.compose_manager.ComposeFile import ComposeFile
 from frappe_manager.compose_project.exceptions import (
+    DockerComposeProjectFailedToPullImagesError,
     DockerComposeProjectFailedToRemoveError,
     DockerComposeProjectFailedToRestartError,
     DockerComposeProjectFailedToStartError,
@@ -77,7 +78,7 @@ def pull_images(self):
             if self.quiet:
                 richprint.live_lines(output, padding=(0, 0, 0, 2))
         except DockerException as e:
-            raise DockerComposeProjectFailedToRemoveError(
+            raise DockerComposeProjectFailedToPullImagesError(
                 self.compose_file_manager.compose_path, self.compose_file_manager.get_services_list()
             )
 

frappe_manager/migration_manager/migration_base.py

@@ -4,23 +4,23 @@
 from frappe_manager.migration_manager.version import Version
 from frappe_manager.logger import log
 
+
 @runtime_checkable
 class MigrationBase(Protocol):
     version: Version = Version("0.0.0")
     skip: bool = False
     migration_executor = None
     backup_manager: BackupManager  # Declare the backup_manager variable
-    logger : Logger
+    logger: Logger
 
     def init(self):
         self.backup_manager = BackupManager(str(self.version))  # Assign the value to backup_manager
-        self.logger =  log.get_logger()
+        self.logger = log.get_logger()
 
     def set_migration_executor(self, migration_executor):
         self.migration_executor = migration_executor
 
     def get_rollback_version(self):
-
         return self.version
 
     def up(self):

frappe_manager/migration_manager/migration_executor.py

@@ -78,7 +78,7 @@ def execute(self):
             richprint.print("Pending Migrations...", emoji_code=':counterclockwise_arrows_button:')
 
             for migration in self.migrations:
-                richprint.print(f"[bold]v{migration.version}[/bold]",emoji_code=':package:')
+                richprint.print(f"[bold]v{migration.version}[/bold]", emoji_code=':package:')
 
             richprint.print("This process may take a while.", emoji_code="\n:hourglass_not_done:")
 
@@ -105,8 +105,10 @@ def execute(self):
         rollback = False
         archive = False
 
+        exception_migration_in_bench_occured = False
         try:
             # run all the migrations
+            prev_migration = None
             for migration in self.migrations:
                 richprint.change_head(f"Running migration introduced in v{migration.version}")
                 self.logger.info(f"[{migration.version}] : Migration starting")
@@ -115,13 +117,16 @@ def execute(self):
 
                     migration.up()
 
-                    if not self.rollback_version > migration.version:
-                        self.rollback_version = migration.get_rollback_version()
+                    prev_migration = migration
+                    if not exception_migration_in_bench_occured:
+                        self.rollback_version = prev_migration.get_rollback_version()
 
                 except MigrationExceptionInBench as e:
-                    captured_output = capture_and_format_exception()
+                    captured_output = capture_and_format_exception(traceback_max_frames=0)
                     self.logger.error(f"[{migration.version}] : Migration Failed\n{captured_output}")
+
                     if migration.version < self.migrations[-1].version:
+                        exception_migration_in_bench_occured = True
                         continue
                     raise e
 
@@ -130,47 +135,56 @@ def execute(self):
                     self.logger.error(f"[{migration.version}] : Migration Failed\n{captured_output}")
                     raise e
 
+            if exception_migration_in_bench_occured:
+                raise MigrationExceptionInBench('')
+
         except MigrationExceptionInBench as e:
             if self.migrate_benches:
                 print_head = True
                 for bench, bench_status in self.migrate_benches.items():
                     if not bench_status["exception"]:
                         if print_head:
-                                richprint.stdout.rule('[bold][red]Migration Passed Benches[/red][bold]',style='green')
-                                print_head = False
+                            richprint.stdout.rule('[bold][red]Migration Passed Benches[/red][bold]', style='green')
+                            print_head = False
 
-                        richprint.print(f"[red]Bench[/red]: {bench}",emoji_code=':construction:')
+                        richprint.print(f"[green]Bench[/green]: {bench}", emoji_code=':construction:')
 
                 if not print_head:
                     richprint.stdout.rule(style='red')
 
                 print_head = True
+
                 for bench, bench_status in self.migrate_benches.items():
                     if bench_status["exception"]:
                         if print_head:
-                                richprint.stdout.rule(':police_car_light: [bold][red]Migration Failed Benches[/red][bold] :police_car_light:',style='red')
-                                print_head = False
+                            richprint.stdout.rule(
+                                ':police_car_light: [bold][red]Migration Failed Benches[/red][bold] :police_car_light:',
+                                style='red',
+                            )
+                            print_head = False
 
-                        richprint.error(f"[red]Bench[/red]: {bench}",emoji_code=':construction:')
+                        richprint.error(f"[red]Bench[/red]: {bench}", emoji_code=':construction:')
 
                         richprint.error(
-                            f"[red]Failed Migration Version[/red]: {bench_status['last_migration_version']}",emoji_code=':package:'
+                            f"[red]Failed Migration Version[/red]: {bench_status['last_migration_version']}",
+                            emoji_code=':package:',
                         )
 
                         richprint.error(
-                            f"[red]Exception[/red]: {type(bench_status['exception']).__name__}",emoji_code=':stop_sign:'
+                            f"[red]Exception[/red]: {type(bench_status['exception']).__name__}",
+                            emoji_code=':stop_sign:',
                         )
-                        richprint.stdout.print(Padding(Text(text=str(bench_status['exception'])),(0,0,0,3)))
+                        richprint.stdout.print(Padding(Text(text=str(bench_status['exception'])), (0, 0, 0, 3)))
 
                 richprint.print(f"For error specifics, refer to {CLI_DIR}/logs/fm.log", emoji_code=':page_facing_up:')
 
                 if not print_head:
                     richprint.stdout.rule(style='red')
 
                 archive_msg = [
-                    'Please select one of the available options after migrations failure :\n',
-                    f"[blue]yes[/blue] Archive failed benches : Benches that have failed will be rolled back and stored in '{CLI_SITES_ARCHIVE}'.",
-                    '[blue]no[/blue] Revert migration : Revert the entire fm enrionment to the previous fm version before migration.',
+                    'Available options after migrations failure :',
+                    f"[blue]\[yes][/blue] Archive failed benches : Benches that have failed will be rolled back to there last successfully completed migration version and stored in '{CLI_SITES_ARCHIVE}'.",
+                    '[blue]\[no][/blue] Revert migration : Restore the FM CLI and FM environment to the last successfully completed migration version for all benches.',
                     '\nDo you wish to archive all benches that failed during migration ?',
                 ]
                 archive = richprint.prompt_ask(prompt="\n".join(archive_msg), choices=["yes", "no"])
@@ -179,7 +193,7 @@ def execute(self):
                     rollback = True
 
         except Exception as e:
-            richprint.print(f"Migration failed: {e}")
+            richprint.error(f"[red]Migration failed[red] : {e}")
             rollback = True
 
         if archive == "yes":
@@ -199,14 +213,18 @@ def execute(self):
                 f"Installing [bold][blue]Frappe-Manager[/blue][/bold] version: v{str(self.rollback_version.version)}"
             )
             install_package("frappe-manager", str(self.rollback_version.version))
-            richprint.exit("Rollback complete.",emoji_code=':back:')
+            richprint.exit("Rollback complete.", emoji_code=':back:')
 
         self.fm_config_manager.version = self.current_version
         self.fm_config_manager.export_to_toml()
         return True
 
     def set_bench_data(
-        self, bench: MigrationBench, exception=None, migration_version: Optional[Version] = None, traceback_str: Optional[str] = None
+        self,
+        bench: MigrationBench,
+        exception=None,
+        migration_version: Optional[Version] = None,
+        traceback_str: Optional[str] = None,
     ):
         self.migrate_benches[bench.name] = {
             "object": bench,

frappe_manager/migration_manager/migrations/migrate_0_13_0.py

@@ -165,6 +165,11 @@ def undo_bench_migrate(self, bench: MigrationBench):
             if backup.bench == bench.name:
                 self.backup_manager.restore(backup, force=True)
 
+        admin_tools_compose_path = bench.path / 'docker-compose.admin-tools.yml'
+
+        if admin_tools_compose_path.exists():
+            admin_tools_compose_path.unlink()
+
         self.logger.info(f"Undo successfull for bench: {bench.name}")
 
     def migrate_bench_compose(self, bench: MigrationBench):

frappe_manager/site_manager/site_exceptions.py

@@ -159,7 +159,7 @@ def __init__(self, bench_name, service_name, message="Attach to {} service conta
 
 
 class BenchNotRunning(BenchException):
-    def __init__(self, bench_name, message="Services not running."):
+    def __init__(self, bench_name, message="Bench services not running."):
         self.bench_name = bench_name
         self.message = message
         super().__init__(self.bench_name, self.message)

frappe_manager/utils/helpers.py

@@ -405,13 +405,15 @@ def rich_traceback_to_string(traceback: Traceback) -> str:
     return captured_str
 
 
-def capture_and_format_exception() -> str:
+def capture_and_format_exception(traceback_max_frames: int = 100) -> str:
     """Capture the current exception and return a formatted traceback string."""
 
     exc_type, exc_value, exc_traceback = sys.exc_info()  # Capture current exception info
     # Create a Traceback object with rich formatting
     #
-    traceback = Traceback.from_exception(exc_type, exc_value, exc_traceback, show_locals=True)
+    traceback = Traceback.from_exception(
+        exc_type, exc_value, exc_traceback, show_locals=True, max_frames=traceback_max_frames
+    )
 
     # Convert the Traceback object to a formatted string
     formatted_traceback = rich_traceback_to_string(traceback)

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "frappe-manager"
-version = "0.13.2"
+version = "0.13.3"
 license = "MIT"
 repository = "https://github.com/rtcamp/frappe-manager"
 description = "A CLI tool based on Docker Compose to easily manage Frappe based projects. As of now, only suitable for development in local machines running on Mac and Linux based OS."