From 11d985ead22963e7fef042fc562289dc3273e721 Mon Sep 17 00:00:00 2001
From: Anton Agestam <anton.agestam@aiven.io>
Date: Fri, 2 Jun 2023 14:49:42 +0200
Subject: [PATCH] fix: update documentation, gracefully handle required arg

---
 karapace/backup/api.py                     |  6 +++---
 karapace/backup/cli.py                     |  8 ++++++--
 tests/integration/backup/test_v3_backup.py | 17 +++++++++++++++++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/karapace/backup/api.py b/karapace/backup/api.py
index 4280efd9a..6e621eda7 100644
--- a/karapace/backup/api.py
+++ b/karapace/backup/api.py
@@ -430,9 +430,9 @@ def create_backup(
         if not records are received within that time and the target offset has not
         been reached an exception is raised. Defaults to one minute.
     :param overwrite: the output file if it exists.
-    :param replication_factor: if specified, metadata will contain this value for
-        replication factor, instead of the one that would be retrieved from
-        the current topic state.
+    :param replication_factor: Value will be stored in metadata, and used when
+        creating topic during restoration. This is required for Version 3 backup,
+        but has no effect on earlier versions, as they don't handle metadata.
 
     :raises Exception: if consumption fails, concrete exception types are unknown,
         see Kafka implementation.
diff --git a/karapace/backup/cli.py b/karapace/backup/cli.py
index d893e8330..01619d12e 100644
--- a/karapace/backup/cli.py
+++ b/karapace/backup/cli.py
@@ -42,8 +42,12 @@ def parse_args() -> argparse.Namespace:
     parser_get.add_argument("--use-format-v3", action="store_true", help="Use experimental V3 backup format.")
     parser_get.add_argument(
         "--replication-factor",
-        help="Use a specified replication factor instead of retrieving it from the current topic state. "
-        "Required for V3 backup format.",
+        help=(
+            "Value will be stored in metadata, and used when creating topic during restoration. This is required for "
+            "V3 backup, but has no effect on earlier versions, as they don't handle metadata."
+        ),
+        # This is hacky, but such is life with argparse.
+        required="--use-format-v3" in sys.argv,
         type=int,
     )
 
diff --git a/tests/integration/backup/test_v3_backup.py b/tests/integration/backup/test_v3_backup.py
index bd4f87ae2..5d5e388c9 100644
--- a/tests/integration/backup/test_v3_backup.py
+++ b/tests/integration/backup/test_v3_backup.py
@@ -220,6 +220,23 @@ def test_roundtrip_from_kafka_state(
     ]
 
 
+def test_errors_when_omitting_replication_factor(config_file: Path) -> None:
+    with pytest.raises(subprocess.CalledProcessError) as exc_info:
+        subprocess.run(
+            [
+                "karapace_schema_backup",
+                "get",
+                "--use-format-v3",
+                f"--config={config_file!s}",
+                "--topic=foo-bar",
+                "--location=foo-bar",
+            ],
+            capture_output=True,
+            check=True,
+        )
+    assert "the following arguments are required: --replication-factor" in exc_info.value.stderr.decode()
+
+
 def test_roundtrip_from_file(
     tmp_path: Path,
     config_file: Path,