From ca14366e389728768c9cdc253adce1fe03c7454b Mon Sep 17 00:00:00 2001 From: jitendra-ky Date: Sat, 16 Nov 2024 18:58:23 +0530 Subject: [PATCH] import: Replace generic Exception with CommandError. This change improves error handling in `do_import_realm` by replacing the use of a generic Exception with CommandError. The updated approach provides clearer, user-friendly error messages when there is a version mismatch between the exported data and the Zulip server. Fixes #32292. --- zerver/lib/import_realm.py | 17 +++++++++-------- .../extra_migrations_error.txt | 6 +++--- .../unapplied_migrations_error.txt | 6 +++--- zerver/tests/test_import_export.py | 9 +++++---- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index 41e0f7782f..3f14c89a9b 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -13,6 +13,7 @@ import pyvips from bs4 import BeautifulSoup from django.conf import settings from django.core.cache import cache +from django.core.management.base import CommandError from django.core.validators import validate_email from django.db import connection, transaction from django.db.backends.utils import CursorWrapper @@ -2122,10 +2123,10 @@ def check_migration_status(exported_migration_status: MigrationStatusJson) -> No exported_primary_version = exported_migration_status["zulip_version"].split(".")[0] local_primary_version = local_migration_status["zulip_version"].split(".")[0] if exported_primary_version != local_primary_version: - raise Exception( - "Export was generated on a different Zulip major version.\n" - f"Export={exported_migration_status['zulip_version']}\n" - f"Server={local_migration_status['zulip_version']}" + raise CommandError( + "Error: Export was generated on a different Zulip major version.\n" + f"Export version: {exported_migration_status['zulip_version']}\n" + f"Server version: {local_migration_status['zulip_version']}" ) exported_migrations_by_app = exported_migration_status["migrations_by_app"] local_migrations_by_app = local_migration_status["migrations_by_app"] @@ -2160,13 +2161,13 @@ def check_migration_status(exported_migration_status: MigrationStatusJson) -> No ] error_message = ( - "Export was generated on a different Zulip version.\n" - f"Export={exported_migration_status['zulip_version']}\n" - f"Server={local_migration_status['zulip_version']}\n" + "Error: Export was generated on a different Zulip version.\n" + f"Export version: {exported_migration_status['zulip_version']}\n" + f"Server version: {local_migration_status['zulip_version']}\n" "\n" "Database formats differ between the exported realm and this server.\n" "Printing migrations that differ between the versions:\n" "--- exported realm\n" "+++ this server" ) + "\n".join(sorted_error_log) - raise Exception(error_message) + raise CommandError(error_message) diff --git a/zerver/tests/fixtures/import_fixtures/check_migrations_errors/extra_migrations_error.txt b/zerver/tests/fixtures/import_fixtures/check_migrations_errors/extra_migrations_error.txt index 2fea357067..7cabeba045 100644 --- a/zerver/tests/fixtures/import_fixtures/check_migrations_errors/extra_migrations_error.txt +++ b/zerver/tests/fixtures/import_fixtures/check_migrations_errors/extra_migrations_error.txt @@ -1,6 +1,6 @@ -Export was generated on a different Zulip version. -Export={version_placeholder} -Server={version_placeholder} +Error: Export was generated on a different Zulip version. +Export version: {version_placeholder} +Server version: {version_placeholder} Database formats differ between the exported realm and this server. Printing migrations that differ between the versions: diff --git a/zerver/tests/fixtures/import_fixtures/check_migrations_errors/unapplied_migrations_error.txt b/zerver/tests/fixtures/import_fixtures/check_migrations_errors/unapplied_migrations_error.txt index b0f6e810e7..0daceabc2b 100644 --- a/zerver/tests/fixtures/import_fixtures/check_migrations_errors/unapplied_migrations_error.txt +++ b/zerver/tests/fixtures/import_fixtures/check_migrations_errors/unapplied_migrations_error.txt @@ -1,6 +1,6 @@ -Export was generated on a different Zulip version. -Export={version_placeholder} -Server={version_placeholder} +Error: Export was generated on a different Zulip version. +Export version: {version_placeholder} +Server version: {version_placeholder} Database formats differ between the exported realm and this server. Printing migrations that differ between the versions: diff --git a/zerver/tests/test_import_export.py b/zerver/tests/test_import_export.py index 4f3df78009..7150a328a5 100644 --- a/zerver/tests/test_import_export.py +++ b/zerver/tests/test_import_export.py @@ -11,6 +11,7 @@ from unittest.mock import patch import orjson from django.conf import settings from django.core.exceptions import ValidationError +from django.core.management.base import CommandError from django.db.models import Q, QuerySet from django.utils.timezone import now as timezone_now from typing_extensions import override @@ -2199,7 +2200,7 @@ class RealmImportExportTest(ExportFile): with ( patch("zerver.lib.import_realm.ZULIP_VERSION", "8.0"), - self.assertRaises(Exception) as e, + self.assertRaises(CommandError) as e, self.assertLogs(level="INFO"), ): do_import_realm( @@ -2207,9 +2208,9 @@ class RealmImportExportTest(ExportFile): "test-zulip", ) expected_error_message = ( - "Export was generated on a different Zulip major version.\n" - f"Export={ZULIP_VERSION}\n" - "Server=8.0" + "Error: Export was generated on a different Zulip major version.\n" + f"Export version: {ZULIP_VERSION}\n" + "Server version: 8.0" ) self.assertEqual(expected_error_message, str(e.exception))