feat(JA): Add force-rehash option to snapshot command. Move Integ Tst…

…s for Diff and Snapshot to Integ

Signed-off-by: David Leong <[email protected]>

src/deadline/client/cli/_groups/click_logger.py

@@ -15,6 +15,12 @@ class ClickLogger:
     def __init__(self, is_json: bool):
         self._is_json = is_json
 
+    def is_json(self) -> bool:
+        """
+        Is logging in JSON mode.
+        """
+        return self._is_json
+
     def echo(
         self,
         message: t.Optional[t.Any] = None,

src/deadline/client/cli/_groups/manifest_group.py

@@ -72,6 +72,12 @@ def cli_manifest():
     help="Include and exclude config of files and directories to include and exclude. Can be a json file or json string.",
     multiple=True,
 )
+@click.option(
+    "--force-rehash",
+    default=False,
+    is_flag=True,
+    help="Rehash all files to compare using file hashes.",
+)
 @click.option("--diff", default=None, help="File Path to Asset Manifest to diff against.")
 @click.option("--json", default=None, is_flag=True, help="Output is printed as JSON for scripting.")
 @_handle_error
@@ -83,6 +89,7 @@ def manifest_snapshot(
     exclude: List[str],
     include_exclude_config: str,
     diff: str,
+    force_rehash: bool,
     json: bool,
     **args,
 ):
@@ -107,6 +114,7 @@ def manifest_snapshot(
         exclude=exclude,
         include_exclude_config=include_exclude_config,
         diff=diff,
+        force_rehash=force_rehash,
         logger=logger,
     )
     if manifest_out:

src/deadline/job_attachments/_diff.py

@@ -136,10 +136,7 @@ def _fast_file_list_to_manifest_diff(
 
     # Select either relative or absolut path for results.
     def select_path(full_path: str, relative_path: str, return_root_relative_path: bool):
-        if return_root_relative_path:
-            return relative_path
-        else:
-            return full_path
+        return relative_path if return_root_relative_path else full_path
 
     changed_paths: List[Tuple[str, FileStatus]] = []
     input_files_map: Dict[str, BaseManifestPath] = {}

src/deadline/job_attachments/api/manifest.py

@@ -213,6 +213,15 @@ def _manifest_diff(
 
     output: ManifestDiff = ManifestDiff()
 
+    # Helper function to update output datastructure.
+    def process_output(status: FileStatus, path: str, output_diff: ManifestDiff):
+        if status == FileStatus.MODIFIED:
+            output_diff.modified.append(path)
+        elif status == FileStatus.NEW:
+            output_diff.new.append(path)
+        elif status == FileStatus.DELETED:
+            output_diff.deleted.append(path)
+
     if force_rehash:
         # hash and create manifest of local directory
         cache_config = config_file.get_cache_directory()
@@ -227,24 +236,14 @@ def _manifest_diff(
         )
         # Map to output datastructure.
         for item in differences:
-            if item[0] == FileStatus.MODIFIED:
-                output.modified.append(item[1].path)
-            elif item[0] == FileStatus.NEW:
-                output.new.append(item[1].path)
-            elif item[0] == FileStatus.DELETED:
-                output.deleted.append(item[1].path)
+            process_output(item[0], item[1].path, output)
 
     else:
         # File based comparisons.
         fast_diff: List[Tuple[str, FileStatus]] = _fast_file_list_to_manifest_diff(
             root=root, current_files=input_files, diff_manifest=local_manifest_object, logger=logger
         )
         for fast_diff_item in fast_diff:
-            if fast_diff_item[1] == FileStatus.MODIFIED:
-                output.modified.append(fast_diff_item[0])
-            elif fast_diff_item[1] == FileStatus.NEW:
-                output.new.append(fast_diff_item[0])
-            elif fast_diff_item[1] == FileStatus.DELETED:
-                output.deleted.append(fast_diff_item[0])
+            process_output(fast_diff_item[1], fast_diff_item[0], output)
 
     return output

src/deadline/job_attachments/asset_manifests/_create_manifest.py

@@ -11,7 +11,9 @@
 
 
 def _create_manifest_for_single_root(
-    files: List[str], root: str, logger: ClickLogger
+    files: List[str],
+    root: str,
+    logger: ClickLogger,
 ) -> Optional[BaseAssetManifest]:
     """
     Shared logic to create a manifest file from a single root.
@@ -38,7 +40,9 @@ def _create_manifest_for_single_root(
             total_input_files=upload_group.total_input_files,
             total_input_bytes=upload_group.total_input_bytes,
             print_function_callback=logger.echo,
-            hashing_progress_callback=hash_callback_manager.callback,
+            hashing_progress_callback=(
+                hash_callback_manager.callback if not logger.is_json() else None
+            ),
         )
 
     if not manifests or len(manifests) == 0:

test/integ/cli/test_cli_attachment.py

@@ -15,7 +15,6 @@
 from deadline_test_fixtures.job_attachment_manager import JobAttachmentManager
 
 from deadline.client.cli import main
-from deadline.client.cli._groups.attachment_group import cli_attachment
 from deadline.job_attachments.asset_manifests import (
     HashAlgorithm,
     hash_data,
@@ -107,8 +106,6 @@ def test_attachment_s3_cross_account_access_denied(self, external_bucket, temp_d
 
         # When - test upload the local asset file
         runner = CliRunner()
-        # Temporary, always add cli_attachment until launched.
-        main.add_command(cli_attachment)
 
         result = runner.invoke(
             main,
@@ -168,8 +165,6 @@ def test_attachment_basic_flow(self, temp_dir, job_attachment_resources, manifes
         s3_root_uri = f"s3://{job_attachment_resources.bucket_name}/{job_attachment_resources.bucket_root_prefix}"
 
         runner = CliRunner()
-        # Temporary, always add cli_attachment until launched.
-        main.add_command(cli_attachment)
 
         # When - test upload the local asset file
         result = runner.invoke(
@@ -255,8 +250,6 @@ def test_attachment_path_mapping_flow(
         s3_root_uri = f"s3://{job_attachment_resources.bucket_name}/{job_attachment_resources.bucket_root_prefix}"
 
         runner = CliRunner()
-        # Temporary, always add cli_attachment until launched.
-        main.add_command(cli_attachment)
 
         # When - test upload the local asset file with path mapping
         result = runner.invoke(

test/integ/cli/test_cli_manifest.py

@@ -6,7 +6,6 @@
 import os
 import json
 from click.testing import CliRunner
-from deadline.client.cli._groups.manifest_group import cli_manifest
 import pytest
 import tempfile
 from deadline.job_attachments.asset_manifests.hash_algorithms import hash_file, HashAlgorithm
@@ -27,7 +26,7 @@
 TEST_SUB_DIR_2 = "subdir2"
 
 
-class TestSnapshot:
+class TestManifestSnapshot:
 
     @pytest.fixture
     def temp_dir(self):
@@ -50,8 +49,67 @@ def test_snapshot_basic(self, temp_dir):
 
         # When
         runner = CliRunner()
-        # Temporary, always add cli_manifest until launched.
-        main.add_command(cli_manifest)
+        result = runner.invoke(
+            main,
+            [
+                "manifest",
+                "snapshot",
+                "--root",
+                root_dir,
+                "--destination",
+                manifest_dir,
+                "--name",
+                "test",
+            ],
+        )
+        assert result.exit_code == 0, f"Non-Zeo exit code, CLI output {result.output}"
+
+        # Then
+        # Check manifest file details to match correct content
+        manifest_files = os.listdir(manifest_dir)
+        assert (
+            len(manifest_files) == 1
+        ), f"Expected exactly one manifest file, but got {len(manifest_files)}"
+        manifest_file_name = manifest_files[0]
+        assert (
+            "test" in manifest_file_name
+        ), f"Expected test in manifest file name, got {manifest_file_name}"
+
+        manifest_file_path = os.path.join(manifest_dir, manifest_file_name)
+
+        with open(manifest_file_path, "r") as f:
+            manifest_data = json.load(f)
+
+        expected_hash = hash_file(file_path, HashAlgorithm("xxh128"))  # hashed with xxh128
+        manifest_data_paths = manifest_data["paths"]
+        assert (
+            len(manifest_data_paths) == 1
+        ), f"Expected exactly one path inside manifest, but got {len(manifest_data_paths)}"
+        assert manifest_data_paths[0]["path"] == TEST_ROOT_FILE
+        assert manifest_data_paths[0]["hash"] == expected_hash
+
+    def test_snapshot_json(self, temp_dir):
+        """
+        Snapshot with a valid root directory containing one file, and no other parameters. Basic test the CLI calls into the API.
+        Deeper testing is done at the API layer.
+        """
+
+        TEST_FILE_CONTENT = "test file content"
+        TEST_ROOT_FILE = "root_file.txt"
+        TEST_ROOT_DIR = "root_dir"
+        TEST_MANIFEST_DIR = "manifest_dir"
+
+        # Given
+        root_dir = os.path.join(temp_dir, TEST_ROOT_DIR)
+        os.makedirs(root_dir)
+        manifest_dir = os.path.join(temp_dir, TEST_MANIFEST_DIR)
+        os.makedirs(manifest_dir)
+        file_path = os.path.join(root_dir, TEST_ROOT_FILE)
+        with open(file_path, "w") as f:
+            f.write(TEST_FILE_CONTENT)
+
+        # When
+        runner = CliRunner()
         result = runner.invoke(
             main,
             [
@@ -91,3 +149,8 @@ def test_snapshot_basic(self, temp_dir):
         ), f"Expected exactly one path inside manifest, but got {len(manifest_data_paths)}"
         assert manifest_data_paths[0]["path"] == TEST_ROOT_FILE
         assert manifest_data_paths[0]["hash"] == expected_hash
+
+        # Since this is JSON, check that we can parse the json
+        manifest_output = json.loads(result.output)
+        assert "manifest" in manifest_output["manifest"]
+        assert manifest_output["manifest"][0] in manifest_file_path

test/unit/deadline_client/cli/test_cli_manifest_diff.py → test/integ/cli/test_cli_manifest_snapshot.py

@@ -7,15 +7,13 @@
 import os
 from pathlib import Path
 from click.testing import CliRunner
-from deadline.client.cli._groups.manifest_group import cli_manifest
 import pytest
 import tempfile
 
 from deadline.client.cli import main
 
 
-@pytest.mark.skip("Random Failure with no credentials on Github")
-class TestSnapshot:
+class TestManifestSnapshot:
 
     @pytest.fixture
     def temp_dir(self):
@@ -41,7 +39,6 @@ def _create_test_manifest(self, tmp_path: str, root_dir: str) -> str:
 
         # When snapshot is called.
         runner = CliRunner()
-        main.add_command(cli_manifest)
         result = runner.invoke(
             main,
             [
@@ -91,7 +88,6 @@ def test_manifest_diff(self, tmp_path: str, json_output: bool):
 
         # When
         runner = CliRunner()
-        main.add_command(cli_manifest)
         args = ["manifest", "diff", "--root", root_dir, "--manifest", manifest]
         if json_output:
             args.append("--json")

test/unit/deadline_client/cli/test_cli_manifest.py

@@ -186,7 +186,6 @@ def test_snapshot_root_directory_only(
         mock_prepare_paths_for_upload.return_value = mock_upload_group
 
         runner = CliRunner()
-        main.add_command(manifest_group.cli_manifest)
         result = runner.invoke(main, ["manifest", "snapshot", "--root", root_dir])
 
         assert result.exit_code == 0
@@ -202,7 +201,6 @@ def test_invalid_root_directory(self, tmp_path):
         invalid_root_dir = str(tmp_path / "invalid_dir")
 
         runner = CliRunner()
-        main.add_command(manifest_group.cli_manifest)
         result = runner.invoke(main, ["manifest", "snapshot", "--root", invalid_root_dir])
 
         assert result.exit_code != 0
@@ -224,7 +222,6 @@ def test_valid_manifest_out(
         mock_prepare_paths_for_upload.return_value = mock_upload_group
 
         runner = CliRunner()
-        main.add_command(manifest_group.cli_manifest)
         result = runner.invoke(
             main,
             [
@@ -249,7 +246,6 @@ def test_invalid_manifest_out(self, tmp_path):
         invalid_manifest_out = str(tmp_path / "nonexistent_dir")
 
         runner = CliRunner()
-        main.add_command(manifest_group.cli_manifest)
         result = runner.invoke(
             main,
             ["manifest", "snapshot", "--root", root_dir, "--destination", invalid_manifest_out],
@@ -276,7 +272,6 @@ def test_asset_snapshot_recursive(
         mock_prepare_paths_for_upload.return_value = mock_upload_group
 
         runner = CliRunner()
-        main.add_command(manifest_group.cli_manifest)
         result = runner.invoke(main, ["manifest", "snapshot", "--root", root_dir])
 
         assert result.exit_code == 0