From 10082679d0756bd69b131fd5ec741c61650fd955 Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Sun, 19 Jul 2026 20:08:50 +0200 Subject: [PATCH 1/6] Fix setup apply failure exit status --- CHANGELOG.md | 1 + .../apache_polaris/cli/command/setup.py | 37 +++++++++++++------ client/python/tests/test_setup_command.py | 27 ++++++++++++++ 3 files changed, 53 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1929e87fdc2..1ed4409a546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti - Deprecated `ALLOW_EXTERNAL_TABLE_LOCATION`. Use `ALLOW_EXTERNAL_METADATA_FILE_LOCATION` for external metadata file locations, including catalog config `polaris.config.allow.external.metadata.file.location`. ### Fixes +- Python CLI `setup apply` now exits with an error after any provisioning operation fails, while still attempting the remaining operations. Previously, individual failures were logged but the command reported success and exited with status 0. - Python CLI `tables list`, `tables get`, and `tables delete` commands now exit with status 1 when catalog API requests fail. Previously, these commands printed an error but exited successfully. - Default table storage locations with object-storage prefixing enabled are now percent-encoded with UTF-8 instead of the JVM default charset. Previously the persisted location of a table under a non-ASCII namespace or table name depended on the platform default charset, so the same table could resolve to a different (or lossy) location on a non-UTF-8 JVM. - Async task execution (table cleanup, manifest and batch file cleanup) now retries when a handler returns false on transient errors (e.g. IO or delete failures). Previously `false` was swallowed with only a warning log and the task was never retried via the existing retry mechanism. diff --git a/client/python/apache_polaris/cli/command/setup.py b/client/python/apache_polaris/cli/command/setup.py index 72bbd5c6b20..0a38e0f140a 100644 --- a/client/python/apache_polaris/cli/command/setup.py +++ b/client/python/apache_polaris/cli/command/setup.py @@ -20,11 +20,11 @@ import logging import yaml import json -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Dict, Optional, List, Any, Set from apache_polaris.cli.command import Command -from apache_polaris.cli.exceptions import CliError +from apache_polaris.cli.exceptions import CliError, CLI_ERROR_EXIT_CODE from apache_polaris.cli.constants import ( PrincipalType, Subcommands, @@ -76,6 +76,7 @@ class SetupCommand(Command): _existing_catalogs: Optional[Set[str]] = None _existing_principals: Optional[Set[str]] = None _catalog_api: Optional[Any] = None + _failure_count: int = field(default=0, init=False, repr=False) def _get_catalog_api(self, api: PolarisDefaultApi) -> Any: """Get or create and cache the IcebergCatalogAPI client.""" @@ -524,6 +525,7 @@ def validate(self) -> None: def execute(self, api: PolarisDefaultApi) -> None: """Execute the setup command based on the subcommand (apply or export).""" if self.setup_subcommand == Subcommands.APPLY: + self._failure_count = 0 if self.dry_run: logger.info("=== Starting Setup Dry-Run ===") else: @@ -574,6 +576,11 @@ def execute(self, api: PolarisDefaultApi) -> None: logger.info(f"--- Finished processing catalog: {catalog_name} ---") if self.dry_run: logger.info("=== Dry-Run Finished ===") + elif self._failure_count: + raise CliError( + f"Setup apply failed; provisioning errors: {self._failure_count}", + exit_code=CLI_ERROR_EXIT_CODE, + ) else: logger.info("=== Setup Apply Process Completed Successfully ===") elif self.setup_subcommand == Subcommands.EXPORT: @@ -601,6 +608,10 @@ def _log_dry_run( message += f" with details:\n{details_str}" logger.info(message) + def _record_failure(self, message: str) -> None: + self._failure_count += 1 + logger.exception(message) + def _create_principals( self, api: PolarisDefaultApi, @@ -640,7 +651,7 @@ def _create_principals( ) existing_principals.add(principal_name) except Exception: - logger.exception( + self._record_failure( f"Failed to create principal '{principal_name}'" ) # Assign roles @@ -681,7 +692,7 @@ def _create_principals( f"Assigned principal '{principal_name}' to role '{role_name}' successfully." ) except Exception: - logger.exception( + self._record_failure( f"Failed to assign principal '{principal_name}' to role '{role_name}'" ) logger.info("--- Finished processing principals ---") @@ -722,7 +733,9 @@ def _create_principal_roles( if self._existing_principal_roles is not None: self._existing_principal_roles.add(role_name) except Exception: - logger.exception(f"Failed to create principal role '{role_name}'") + self._record_failure( + f"Failed to create principal role '{role_name}'" + ) logger.info("--- Finished processing principal roles ---") def _map_storage_properties(self, catalog_data: Dict[str, Any]) -> Dict[str, Any]: @@ -954,7 +967,7 @@ def _create_catalogs( logger.info(f"Catalog '{catalog_name}' created successfully.") existing_catalogs.add(catalog_name) except Exception: - logger.exception(f"Failed to create catalog '{catalog_name}'") + self._record_failure(f"Failed to create catalog '{catalog_name}'") overall_success = False logger.info("--- Finished processing catalogs ---") return overall_success @@ -1001,7 +1014,7 @@ def _create_catalog_roles( # Update the cache with the newly created role existing_roles_in_catalog.add(role_name) except Exception: - logger.exception( + self._record_failure( f"Failed to create catalog role '{role_name}' in catalog '{catalog_name}'" ) continue @@ -1040,7 +1053,7 @@ def _create_catalog_roles( f"Assigned catalog role '{role_name}' to principal role '{principal_role_name}' successfully." ) except Exception: - logger.exception( + self._record_failure( f"Failed to assign catalog role '{role_name}' to principal role '{principal_role_name}'" ) # Grant privileges @@ -1115,7 +1128,7 @@ def _grant_privilege( cmd.execute(api) logger.info(f"Successfully granted {log_message}") except Exception: - logger.exception(f"Failed to grant {log_message}") + self._record_failure(f"Failed to grant {log_message}") def _create_namespaces( self, @@ -1216,7 +1229,7 @@ def _create_namespaces( ) existing_namespaces.add(ns_name) except Exception: - logger.exception( + self._record_failure( f"Failed to create namespace '{ns_name}' in catalog '{catalog_name}'" ) logger.info( @@ -1314,7 +1327,7 @@ def _create_policies_and_attachments( ) logger.info(f"Policy '{policy_name}' created successfully.") except Exception: - logger.exception(f"Failed to create policy '{policy_name}'") + self._record_failure(f"Failed to create policy '{policy_name}'") continue # Attachments attachments = policy_data.get("attach", []) @@ -1355,7 +1368,7 @@ def _create_policies_and_attachments( cmd.execute(api) logger.info(f"Successfully attached policy '{policy_name}'") except Exception: - logger.exception(f"Failed to attach policy '{policy_name}'") + self._record_failure(f"Failed to attach policy '{policy_name}'") logger.info(f"--- Finished processing policies for catalog: {catalog_name} ---") def _validate_entity( diff --git a/client/python/tests/test_setup_command.py b/client/python/tests/test_setup_command.py index 7a849c77450..0a9d27824cf 100644 --- a/client/python/tests/test_setup_command.py +++ b/client/python/tests/test_setup_command.py @@ -20,6 +20,7 @@ import io from unittest.mock import patch, MagicMock, mock_open from cli_test_utils import CLITestBase, INVALID_ARGS +from apache_polaris.cli.exceptions import CliError, CLI_ERROR_EXIT_CODE from apache_polaris.sdk.management import ( PolarisCatalog, CatalogProperties, @@ -66,6 +67,32 @@ def test_setup_apply_s3_optional_fields(self, mock_isfile: MagicMock) -> None: self.assertIsNone(call_args.catalog.storage_config_info.role_arn) self.assertEqual(call_args.catalog.name, "s3-catalog") + @patch("apache_polaris.cli.command.setup.os.path.isfile") + def test_setup_apply_reports_provisioning_failures( + self, mock_isfile: MagicMock + ) -> None: + mock_client = self.build_mock_client() + mock_isfile.return_value = True + mock_client.list_principal_roles.return_value.roles = [] + mock_client.create_principal_role.side_effect = [ + RuntimeError("backend unavailable"), + None, + ] + setup_yaml = "principal_roles:\n - failed-role\n - successful-role" + + with ( + patch( + "apache_polaris.cli.command.setup.open", + mock_open(read_data=setup_yaml), + ), + self.assertRaises(CliError) as cm, + ): + self.mock_execute(mock_client, ["setup", "apply", "config.yaml"]) + + self.assertEqual(cm.exception.exit_code, CLI_ERROR_EXIT_CODE) + self.assertIn("provisioning errors: 1", str(cm.exception)) + self.assertEqual(mock_client.create_principal_role.call_count, 2) + @patch( "apache_polaris.cli.command.setup.open", new_callable=mock_open, read_data="{}" ) From e27a9f7b97c5e3dc1ae0858a7b8823c5c991369b Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Sun, 19 Jul 2026 23:19:54 +0200 Subject: [PATCH 2/6] Count setup discovery failures --- CHANGELOG.md | 2 +- .../apache_polaris/cli/command/setup.py | 34 ++++++++----- client/python/tests/test_setup_command.py | 49 ++++++++++++++++++- 3 files changed, 71 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ed4409a546..a6cfd625d6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,7 +88,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti - Deprecated `ALLOW_EXTERNAL_TABLE_LOCATION`. Use `ALLOW_EXTERNAL_METADATA_FILE_LOCATION` for external metadata file locations, including catalog config `polaris.config.allow.external.metadata.file.location`. ### Fixes -- Python CLI `setup apply` now exits with an error after any provisioning operation fails, while still attempting the remaining operations. Previously, individual failures were logged but the command reported success and exited with status 0. +- Python CLI `setup apply` now exits with an error after any setup operation fails, while still attempting the remaining operations. Previously, individual failures were logged but the command reported success and exited with status 0. - Python CLI `tables list`, `tables get`, and `tables delete` commands now exit with status 1 when catalog API requests fail. Previously, these commands printed an error but exited successfully. - Default table storage locations with object-storage prefixing enabled are now percent-encoded with UTF-8 instead of the JVM default charset. Previously the persisted location of a table under a non-ASCII namespace or table name depended on the platform default charset, so the same table could resolve to a different (or lossy) location on a non-UTF-8 JVM. - Async task execution (table cleanup, manifest and batch file cleanup) now retries when a handler returns false on transient errors (e.g. IO or delete failures). Previously `false` was swallowed with only a warning log and the task was never retried via the existing retry mechanism. diff --git a/client/python/apache_polaris/cli/command/setup.py b/client/python/apache_polaris/cli/command/setup.py index 0a38e0f140a..e955da055f4 100644 --- a/client/python/apache_polaris/cli/command/setup.py +++ b/client/python/apache_polaris/cli/command/setup.py @@ -92,7 +92,7 @@ def _get_existing_principals(self, api: PolarisDefaultApi) -> Set[str]: p.name for p in api.list_principals().principals } except Exception: - logger.exception("Failed to fetch existing principals") + self._record_failure("Failed to fetch existing principals") self._existing_principals = set() return self._existing_principals @@ -104,7 +104,7 @@ def _get_existing_principal_roles(self, api: PolarisDefaultApi) -> Set[str]: role.name for role in api.list_principal_roles().roles } except Exception: - logger.exception("Failed to fetch existing principal roles") + self._record_failure("Failed to fetch existing principal roles") self._existing_principal_roles = set() return self._existing_principal_roles @@ -126,8 +126,8 @@ def _get_existing_catalog_roles( # In dry-run, a 404 is expected if the catalog doesn't exist yet is_404 = getattr(e, "status", None) == 404 or "(404)" in str(e) if not (self.dry_run and is_404): - logger.warning( - f"Failed to fetch catalog roles for catalog '{catalog_name}': {e}" + self._record_failure( + f"Failed to fetch catalog roles for catalog '{catalog_name}'" ) self._existing_catalog_roles[catalog_name] = set() return self._existing_catalog_roles[catalog_name] @@ -139,7 +139,7 @@ def _get_existing_catalogs(self, api: PolarisDefaultApi) -> Set[str]: try: self._existing_catalogs = {c.name for c in api.list_catalogs().catalogs} except Exception: - logger.exception("Failed to fetch existing catalogs") + self._record_failure("Failed to fetch existing catalogs") self._existing_catalogs = set() return self._existing_catalogs @@ -574,13 +574,13 @@ def execute(self, api: PolarisDefaultApi) -> None: dry_run=self.dry_run, ) logger.info(f"--- Finished processing catalog: {catalog_name} ---") - if self.dry_run: - logger.info("=== Dry-Run Finished ===") - elif self._failure_count: + if self._failure_count: raise CliError( - f"Setup apply failed; provisioning errors: {self._failure_count}", + f"Setup apply failed; setup errors: {self._failure_count}", exit_code=CLI_ERROR_EXIT_CODE, ) + elif self.dry_run: + logger.info("=== Dry-Run Finished ===") else: logger.info("=== Setup Apply Process Completed Successfully ===") elif self.setup_subcommand == Subcommands.EXPORT: @@ -620,6 +620,9 @@ def _create_principals( ) -> None: """Create principals and assign them to principal roles.""" logger.info("--- Processing principals ---") + if not principals_config: + logger.info("--- Finished processing principals ---") + return existing_principals = self._get_existing_principals(api) for principal_name, principal_data in principals_config.items(): if principal_name in existing_principals: @@ -705,6 +708,9 @@ def _create_principal_roles( ) -> None: """Create principal roles.""" logger.info("--- Processing principal roles ---") + if not principal_roles_config: + logger.info("--- Finished processing principal roles ---") + return self._get_existing_principal_roles(api) for role_name in principal_roles_config: @@ -981,8 +987,14 @@ def _create_catalog_roles( ) -> None: """Create catalog roles, assign them to principal roles, and grant privileges.""" logger.info(f"--- Processing catalog roles for catalog: {catalog_name} ---") + if not roles_config: + logger.info( + f"--- Finished processing catalog roles for catalog: {catalog_name} ---" + ) + return existing_roles_in_catalog = self._get_existing_catalog_roles(api, catalog_name) - self._get_existing_principal_roles(api) + if any(role_data.get("assign_to") for role_data in roles_config.values()): + self._get_existing_principal_roles(api) for role_name, role_data in roles_config.items(): if role_name in existing_roles_in_catalog: logger.info( @@ -1189,7 +1201,7 @@ def _create_namespaces( existing_namespaces.add(".".join(ns)) listed_parents.add(parent_ns) except Exception: - logger.exception( + self._record_failure( f"Failed to list sub-namespaces for '{parent_ns}'" ) diff --git a/client/python/tests/test_setup_command.py b/client/python/tests/test_setup_command.py index 0a9d27824cf..d411559ab8b 100644 --- a/client/python/tests/test_setup_command.py +++ b/client/python/tests/test_setup_command.py @@ -20,6 +20,8 @@ import io from unittest.mock import patch, MagicMock, mock_open from cli_test_utils import CLITestBase, INVALID_ARGS +from apache_polaris.cli.command.setup import SetupCommand +from apache_polaris.cli.constants import Subcommands from apache_polaris.cli.exceptions import CliError, CLI_ERROR_EXIT_CODE from apache_polaris.sdk.management import ( PolarisCatalog, @@ -51,6 +53,44 @@ def test_setup_apply_dry_run( self.mock_execute(mock_client, ["setup", "apply", "config.yaml", "--dry-run"]) mock_client.list_principals.assert_called() + @patch("apache_polaris.cli.command.setup.os.path.isfile") + def test_setup_dry_run_reports_lookup_failures( + self, mock_isfile: MagicMock + ) -> None: + mock_client = self.build_mock_client() + mock_isfile.return_value = True + mock_client.list_principals.side_effect = RuntimeError("listing unavailable") + setup_yaml = "principals:\n quickstart_user: {}" + + with ( + patch( + "apache_polaris.cli.command.setup.open", + mock_open(read_data=setup_yaml), + ), + self.assertRaises(CliError) as cm, + ): + self.mock_execute( + mock_client, + ["setup", "apply", "config.yaml", "--dry-run"], + ) + + self.assertEqual(cm.exception.exit_code, CLI_ERROR_EXIT_CODE) + self.assertIn("setup errors: 1", str(cm.exception)) + mock_client.create_principal.assert_not_called() + + def test_setup_dry_run_ignores_missing_catalog_roles(self) -> None: + mock_client = self.build_mock_client() + mock_client.list_catalog_roles.side_effect = RuntimeError("(404)") + command = SetupCommand( + setup_subcommand=Subcommands.APPLY, + dry_run=True, + ) + + self.assertEqual( + command._get_existing_catalog_roles(mock_client, "new-catalog"), set() + ) + self.assertEqual(command._failure_count, 0) + @patch("apache_polaris.cli.command.setup.os.path.isfile") def test_setup_apply_s3_optional_fields(self, mock_isfile: MagicMock) -> None: mock_client = self.build_mock_client() @@ -66,6 +106,9 @@ def test_setup_apply_s3_optional_fields(self, mock_isfile: MagicMock) -> None: # role_arn should be None, NOT an empty string self.assertIsNone(call_args.catalog.storage_config_info.role_arn) self.assertEqual(call_args.catalog.name, "s3-catalog") + mock_client.list_principals.assert_not_called() + mock_client.list_principal_roles.assert_not_called() + mock_client.list_catalog_roles.assert_not_called() @patch("apache_polaris.cli.command.setup.os.path.isfile") def test_setup_apply_reports_provisioning_failures( @@ -73,7 +116,9 @@ def test_setup_apply_reports_provisioning_failures( ) -> None: mock_client = self.build_mock_client() mock_isfile.return_value = True - mock_client.list_principal_roles.return_value.roles = [] + mock_client.list_principal_roles.side_effect = RuntimeError( + "listing unavailable" + ) mock_client.create_principal_role.side_effect = [ RuntimeError("backend unavailable"), None, @@ -90,7 +135,7 @@ def test_setup_apply_reports_provisioning_failures( self.mock_execute(mock_client, ["setup", "apply", "config.yaml"]) self.assertEqual(cm.exception.exit_code, CLI_ERROR_EXIT_CODE) - self.assertIn("provisioning errors: 1", str(cm.exception)) + self.assertIn("setup errors: 2", str(cm.exception)) self.assertEqual(mock_client.create_principal_role.call_count, 2) @patch( From 8585e4a21052607997732af4bf42d979d06c10f3 Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Sun, 19 Jul 2026 23:29:45 +0200 Subject: [PATCH 3/6] Count remaining setup apply failures --- .../apache_polaris/cli/command/setup.py | 5 +- client/python/tests/test_setup_command.py | 49 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/client/python/apache_polaris/cli/command/setup.py b/client/python/apache_polaris/cli/command/setup.py index e955da055f4..2fea23beb73 100644 --- a/client/python/apache_polaris/cli/command/setup.py +++ b/client/python/apache_polaris/cli/command/setup.py @@ -902,6 +902,7 @@ def _create_catalogs( logger.warning( f"External catalog '{catalog_name}' is missing the required 'connection' info block." ) + self._failure_count += 1 overall_success = False continue command_args.update(conn_args) @@ -1279,8 +1280,8 @@ def _create_policies_and_attachments( except NotFoundException: policy_exists = False except Exception: - logger.warning( - f"Could not verify existence of policy '{policy_name}', attempting creation." + self._record_failure( + f"Could not verify existence of policy '{policy_name}'" ) policy_exists = False if policy_exists: diff --git a/client/python/tests/test_setup_command.py b/client/python/tests/test_setup_command.py index d411559ab8b..69feb32b785 100644 --- a/client/python/tests/test_setup_command.py +++ b/client/python/tests/test_setup_command.py @@ -91,6 +91,34 @@ def test_setup_dry_run_ignores_missing_catalog_roles(self) -> None: ) self.assertEqual(command._failure_count, 0) + @patch("apache_polaris.cli.command.setup.PolicyAPI") + def test_setup_dry_run_reports_policy_lookup_failures( + self, mock_policy_api: MagicMock + ) -> None: + mock_client = self.build_mock_client() + mock_policy_api.return_value.load_policy.side_effect = RuntimeError( + "listing unavailable" + ) + command = SetupCommand( + setup_subcommand=Subcommands.APPLY, + dry_run=True, + ) + + command._create_policies_and_attachments( + mock_client, + "catalog", + { + "policy": { + "namespace": "namespace", + "type": "data-compaction", + "content": {}, + } + }, + dry_run=True, + ) + + self.assertEqual(command._failure_count, 1) + @patch("apache_polaris.cli.command.setup.os.path.isfile") def test_setup_apply_s3_optional_fields(self, mock_isfile: MagicMock) -> None: mock_client = self.build_mock_client() @@ -110,6 +138,27 @@ def test_setup_apply_s3_optional_fields(self, mock_isfile: MagicMock) -> None: mock_client.list_principal_roles.assert_not_called() mock_client.list_catalog_roles.assert_not_called() + @patch("apache_polaris.cli.command.setup.os.path.isfile") + def test_setup_apply_reports_missing_external_catalog_connection( + self, mock_isfile: MagicMock + ) -> None: + mock_client = self.build_mock_client() + mock_isfile.return_value = True + setup_yaml = "catalogs:\n - name: broken\n type: external" + + with ( + patch( + "apache_polaris.cli.command.setup.open", + mock_open(read_data=setup_yaml), + ), + self.assertRaises(CliError) as cm, + ): + self.mock_execute(mock_client, ["setup", "apply", "config.yaml"]) + + self.assertEqual(cm.exception.exit_code, CLI_ERROR_EXIT_CODE) + self.assertIn("setup errors: 1", str(cm.exception)) + mock_client.create_catalog.assert_not_called() + @patch("apache_polaris.cli.command.setup.os.path.isfile") def test_setup_apply_reports_provisioning_failures( self, mock_isfile: MagicMock From f4f12988fbc4d28ff5c530c0bac96e3e5eb56053 Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Sun, 19 Jul 2026 23:33:52 +0200 Subject: [PATCH 4/6] Count skipped setup role assignments --- .../apache_polaris/cli/command/setup.py | 2 ++ client/python/tests/test_setup_command.py | 32 ++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/client/python/apache_polaris/cli/command/setup.py b/client/python/apache_polaris/cli/command/setup.py index 2fea23beb73..e7e51210f2f 100644 --- a/client/python/apache_polaris/cli/command/setup.py +++ b/client/python/apache_polaris/cli/command/setup.py @@ -671,6 +671,7 @@ def _create_principals( logger.warning( f"Skipping assignment for non-existing principal role '{role_name}' for principal '{principal_name}'" ) + self._failure_count += 1 continue if dry_run: @@ -1041,6 +1042,7 @@ def _create_catalog_roles( logger.warning( f"Skipping assignment of catalog role '{role_name}' to non-existing principal role '{principal_role_name}'" ) + self._failure_count += 1 continue if dry_run: self._log_dry_run( diff --git a/client/python/tests/test_setup_command.py b/client/python/tests/test_setup_command.py index 69feb32b785..bbf1c0be662 100644 --- a/client/python/tests/test_setup_command.py +++ b/client/python/tests/test_setup_command.py @@ -42,7 +42,14 @@ def test_setup_validation(self) -> None: @patch( "apache_polaris.cli.command.setup.open", new_callable=mock_open, - read_data="principals:\n quickstart_user:\n roles:\n - quickstart_user_role", + read_data=( + "principal_roles:\n" + " - quickstart_user_role\n" + "principals:\n" + " quickstart_user:\n" + " roles:\n" + " - quickstart_user_role" + ), ) @patch("apache_polaris.cli.command.setup.os.path.isfile") def test_setup_apply_dry_run( @@ -78,6 +85,29 @@ def test_setup_dry_run_reports_lookup_failures( self.assertIn("setup errors: 1", str(cm.exception)) mock_client.create_principal.assert_not_called() + @patch("apache_polaris.cli.command.setup.os.path.isfile") + def test_setup_dry_run_reports_missing_role_assignments( + self, mock_isfile: MagicMock + ) -> None: + mock_client = self.build_mock_client() + mock_isfile.return_value = True + setup_yaml = "principals:\n user:\n roles:\n - missing-role" + + with ( + patch( + "apache_polaris.cli.command.setup.open", + mock_open(read_data=setup_yaml), + ), + self.assertRaises(CliError) as cm, + ): + self.mock_execute( + mock_client, + ["setup", "apply", "config.yaml", "--dry-run"], + ) + + self.assertEqual(cm.exception.exit_code, CLI_ERROR_EXIT_CODE) + self.assertIn("setup errors: 1", str(cm.exception)) + def test_setup_dry_run_ignores_missing_catalog_roles(self) -> None: mock_client = self.build_mock_client() mock_client.list_catalog_roles.side_effect = RuntimeError("(404)") From 74b5879440d78c60de0bcd1043561693df554327 Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Mon, 20 Jul 2026 10:02:06 +0200 Subject: [PATCH 5/6] Recover successful policy creation after lookup errors --- .../apache_polaris/cli/command/setup.py | 13 ++++++--- client/python/tests/test_setup_command.py | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/client/python/apache_polaris/cli/command/setup.py b/client/python/apache_polaris/cli/command/setup.py index e7e51210f2f..603816d109b 100644 --- a/client/python/apache_polaris/cli/command/setup.py +++ b/client/python/apache_polaris/cli/command/setup.py @@ -525,7 +525,6 @@ def validate(self) -> None: def execute(self, api: PolarisDefaultApi) -> None: """Execute the setup command based on the subcommand (apply or export).""" if self.setup_subcommand == Subcommands.APPLY: - self._failure_count = 0 if self.dry_run: logger.info("=== Starting Setup Dry-Run ===") else: @@ -1282,9 +1281,15 @@ def _create_policies_and_attachments( except NotFoundException: policy_exists = False except Exception: - self._record_failure( - f"Could not verify existence of policy '{policy_name}'" - ) + if dry_run: + self._record_failure( + f"Could not verify existence of policy '{policy_name}'" + ) + else: + logger.warning( + f"Could not verify existence of policy '{policy_name}', attempting creation.", + exc_info=True, + ) policy_exists = False if policy_exists: logger.info( diff --git a/client/python/tests/test_setup_command.py b/client/python/tests/test_setup_command.py index bbf1c0be662..54bd417cbbc 100644 --- a/client/python/tests/test_setup_command.py +++ b/client/python/tests/test_setup_command.py @@ -149,6 +149,35 @@ def test_setup_dry_run_reports_policy_lookup_failures( self.assertEqual(command._failure_count, 1) + @patch("apache_polaris.cli.command.setup.PolicyAPI") + def test_setup_apply_recovers_policy_lookup_failure( + self, mock_policy_api: MagicMock + ) -> None: + mock_client = self.build_mock_client() + mock_policy_api.return_value.load_policy.side_effect = RuntimeError( + "listing unavailable" + ) + command = SetupCommand( + setup_subcommand=Subcommands.APPLY, + dry_run=False, + ) + + command._create_policies_and_attachments( + mock_client, + "catalog", + { + "policy": { + "namespace": "namespace", + "type": "data-compaction", + "content": {}, + } + }, + dry_run=False, + ) + + self.assertEqual(command._failure_count, 0) + mock_policy_api.return_value.create_policy.assert_called_once() + @patch("apache_polaris.cli.command.setup.os.path.isfile") def test_setup_apply_s3_optional_fields(self, mock_isfile: MagicMock) -> None: mock_client = self.build_mock_client() From b7964e509ac793210b8cc0919c5cd10f47623709 Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Tue, 21 Jul 2026 19:44:44 +0200 Subject: [PATCH 6/6] Document and test successful setup apply --- client/python/apache_polaris/cli/command/setup.py | 1 + client/python/tests/test_setup_command.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/client/python/apache_polaris/cli/command/setup.py b/client/python/apache_polaris/cli/command/setup.py index 603816d109b..1f0831995d3 100644 --- a/client/python/apache_polaris/cli/command/setup.py +++ b/client/python/apache_polaris/cli/command/setup.py @@ -1281,6 +1281,7 @@ def _create_policies_and_attachments( except NotFoundException: policy_exists = False except Exception: + # A real apply's create attempt determines whether this failure is terminal. if dry_run: self._record_failure( f"Could not verify existence of policy '{policy_name}'" diff --git a/client/python/tests/test_setup_command.py b/client/python/tests/test_setup_command.py index 54bd417cbbc..d7f075b4186 100644 --- a/client/python/tests/test_setup_command.py +++ b/client/python/tests/test_setup_command.py @@ -60,6 +60,20 @@ def test_setup_apply_dry_run( self.mock_execute(mock_client, ["setup", "apply", "config.yaml", "--dry-run"]) mock_client.list_principals.assert_called() + def test_setup_apply_succeeds_without_failures(self) -> None: + mock_client = self.build_mock_client() + mock_client.list_principal_roles.return_value.roles = [] + command = SetupCommand( + setup_subcommand=Subcommands.APPLY, + setup_config="config.yaml", + _config_cache={"principal_roles": ["successful-role"]}, + ) + + command.execute(mock_client) + + self.assertEqual(command._failure_count, 0) + mock_client.create_principal_role.assert_called_once() + @patch("apache_polaris.cli.command.setup.os.path.isfile") def test_setup_dry_run_reports_lookup_failures( self, mock_isfile: MagicMock