From 06d0f17ce1776ca452d78766b99a2a2de781bebd Mon Sep 17 00:00:00 2001 From: Riyaz Shiraguppi Date: Tue, 12 May 2026 13:02:28 -0500 Subject: [PATCH] feat(import): add --minimize-reads for type-scoped state loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the --minimize-reads option from sync onto import. The flag, validation, and loading-strategy code already live in shared code (utils/configuration.py:412-450, utils/state.py:88-90) — only the click decorator needed adding to the import subcommand. When enabled, import scopes the bucket state-load to the requested resource types (type-scoped) or to filter-extracted exact IDs (ID-targeted), instead of scanning every file under both source and destination prefixes. For populated buckets where state-load dominates per-run wall-clock, this is a significant speedup. Constraints (unchanged, enforced in configuration.py:412-417): - requires --resource-per-file - requires --resources The "must not be combined with --cleanup" rule on sync's flag is enforced by sync's validation path because sync accepts --cleanup. The import command does not register --cleanup at all (it lives in _diffs_options, applied only to sync/diffs), so click rejects "import --cleanup=..." at parse time, before configuration validation runs. A behavioural test pins this invariant down. Tests: - flipped pre-existing test_minimize_reads_not_available_on_import_command to positive test_minimize_reads_accepted_on_import_command (now in --help). - added test_import_minimize_reads_requires_resource_per_file and test_import_minimize_reads_requires_resources_flag, mirroring the sync-side validation tests. - added test_import_does_not_accept_cleanup_option that asserts on the Error: line specifically, so a future click message reword doesn't silently mask a regression that accepts --cleanup on import. - removed the now-stale "Only available on the sync command." line from sync's --minimize-reads help text. - import's --minimize-reads help omits the --cleanup caveat (since --cleanup isn't an import flag, mentioning it would be misleading). No behavioural change for the sync command. No new shared options. --- datadog_sync/commands/_import.py | 15 ++++- datadog_sync/commands/sync.py | 3 +- tests/unit/test_minimize_reads_type_scoped.py | 60 +++++++++++++++++-- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/datadog_sync/commands/_import.py b/datadog_sync/commands/_import.py index b995796f..5cda1c5d 100644 --- a/datadog_sync/commands/_import.py +++ b/datadog_sync/commands/_import.py @@ -3,9 +3,10 @@ # This product includes software developed at Datadog (https://www.datadoghq.com/). # Copyright 2019 Datadog, Inc. -from click import command +from click import command, option from datadog_sync.commands.shared.options import ( + CustomOptionClass, common_options, destination_auth_options, force_missing_dependencies_options, @@ -22,6 +23,18 @@ @common_options @force_missing_dependencies_options @storage_options +@option( + "--minimize-reads", + required=False, + is_flag=True, + default=False, + show_default=True, + help="Minimize cloud storage reads by loading only needed resources. " + "Uses ID-targeted fetching when filters specify exact IDs, " + "otherwise falls back to type-scoped loading. " + "Requires --resource-per-file and --resources.", + cls=CustomOptionClass, +) def _import(**kwargs): """Import Datadog resources.""" run_cmd(Command.IMPORT, **kwargs) diff --git a/datadog_sync/commands/sync.py b/datadog_sync/commands/sync.py index c2e27ace..f8afa9dc 100644 --- a/datadog_sync/commands/sync.py +++ b/datadog_sync/commands/sync.py @@ -37,8 +37,7 @@ "Uses ID-targeted fetching when filters specify exact IDs, " "otherwise falls back to type-scoped loading. " "Requires --resource-per-file and --resources. " - "Must not be combined with --cleanup. " - "Only available on the sync command.", + "Must not be combined with --cleanup.", cls=CustomOptionClass, ) def sync(**kwargs): diff --git a/tests/unit/test_minimize_reads_type_scoped.py b/tests/unit/test_minimize_reads_type_scoped.py index bd7c3ccd..8149fe0c 100644 --- a/tests/unit/test_minimize_reads_type_scoped.py +++ b/tests/unit/test_minimize_reads_type_scoped.py @@ -134,14 +134,66 @@ def test_minimize_reads_not_available_on_diffs_command(self, runner): assert result.exit_code != 0 assert "no such option" in result.output.lower() - def test_minimize_reads_not_available_on_import_command(self, runner): - """--minimize-reads must not be accepted by the import command.""" + def test_minimize_reads_accepted_on_import_command(self, runner): + """--minimize-reads must be accepted by the import command and listed in --help.""" + result = runner.invoke(cli, ["import", "--help"]) + assert result.exit_code == 0 + assert "--minimize-reads" in result.output + + def test_import_minimize_reads_requires_resource_per_file(self, runner): + """--minimize-reads on import without --resource-per-file must fail with error.""" result = runner.invoke( cli, - ["import", "--minimize-reads", "--source-api-key=x"], + [ + "import", + "--minimize-reads", + "--resources=roles", + "--source-api-key=x", + "--source-app-key=y", + ], ) assert result.exit_code != 0 - assert "no such option" in result.output.lower() + assert "resource-per-file" in (result.output + str(result.exception)).lower() + + def test_import_minimize_reads_requires_resources_flag(self, runner): + """--minimize-reads on import without --resources must fail with error.""" + result = runner.invoke( + cli, + [ + "import", + "--minimize-reads", + "--resource-per-file", + "--source-api-key=x", + "--source-app-key=y", + ], + ) + assert result.exit_code != 0 + assert "resources" in (result.output + str(result.exception)).lower() + + def test_import_does_not_accept_cleanup_option(self, runner): + """Pin: import does not register --cleanup, so click rejects it at parse time + and the --minimize-reads + --cleanup combination is unreachable via the import CLI.""" + result = runner.invoke( + cli, + [ + "import", + "--minimize-reads", + "--resource-per-file", + "--resources=roles", + "--cleanup=Force", + "--source-api-key=x", + "--source-app-key=y", + ], + ) + assert result.exit_code != 0 + # Assert on the Error: line only, so a future click message change that drops + # "no such option" or "cleanup" is loud, and a regression that ACCEPTS --cleanup + # on import (no Error: line at all) is loud too. + error_lines = [line for line in result.output.splitlines() if line.lower().startswith("error:")] + assert error_lines, "expected click to emit an Error: line for unknown option" + joined_error = " ".join(error_lines).lower() + assert "no such option" in joined_error + assert "cleanup" in joined_error def test_minimize_reads_cannot_be_combined_with_cleanup(self, runner): """--minimize-reads + --cleanup must be rejected before any I/O."""