merge conflicting branches

Signed-off-by: Tang <[email protected]>

src/deadline/client/cli/_common.py

@@ -9,6 +9,7 @@
     "_handle_error",
     "_apply_cli_options_to_config",
     "_cli_object_repr",
+    "_ProgressBarCallbackManager",
 ]
 
 import sys

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

@@ -7,10 +7,6 @@
     * diff
     * download
 """
-<<<<<<< HEAD
-=======
-
->>>>>>> ae6cbf17fcbeac9714ca0cb1f0924f5531bd3315
 import os
 from pathlib import Path
 
@@ -20,11 +16,9 @@
 from deadline.job_attachments.upload import S3AssetManager, S3AssetUploader
 from deadline.job_attachments.models import JobAttachmentS3Settings
 
-from .._common import _apply_cli_options_to_config, _handle_error, _ProgressBarCallbackManager
+from .._common import _handle_error, _ProgressBarCallbackManager
 from ...exceptions import NonValidInputError
 
-IGNORE_FILE: str = "manifests"
-
 
 @click.group(name="asset")
 @_handle_error
@@ -36,7 +30,9 @@ def cli_asset():
 
 @cli_asset.command(name="snapshot")
 @click.option("--root-dir", required=True, help="The root directory to snapshot. ")
-@click.option("--manifest-out", help="Destination path to directory where manifest is created. ")
+@click.option(
+    "--manifest-out", default=None, help="Destination path to directory where manifest is created. "
+)
 @click.option(
     "--recursive",
     "-r",
@@ -46,31 +42,22 @@ def cli_asset():
     default=False,
 )
 @_handle_error
-def asset_snapshot(root_dir, manifest_out, recursive, **args):
+def asset_snapshot(root_dir: str, manifest_out: str, recursive: bool, **args):
     """
     Creates manifest of files specified root directory.
     """
-    root_dir_basename = os.path.basename(root_dir) + "_"
-
     if not os.path.isdir(root_dir):
-        misconfigured_directories_msg = f"Specified root directory {root_dir} does not exist. "
-        raise NonValidInputError(misconfigured_directories_msg)
+        raise NonValidInputError(f"Specified root directory {root_dir} does not exist. ")
 
     if manifest_out and not os.path.isdir(manifest_out):
-        misconfigured_directories_msg = (
-            f"Specified destination directory {manifest_out} does not exist. "
-        )
-        raise NonValidInputError(misconfigured_directories_msg)
+        raise NonValidInputError(f"Specified destination directory {manifest_out} does not exist. ")
     elif manifest_out is None:
         manifest_out = root_dir
+        click.echo(f"Manifest creation path defaulted to {root_dir} \n")
 
     inputs = []
     for root, dirs, files in os.walk(root_dir):
-        if os.path.basename(root).endswith("_manifests"):
-            continue
-        for file in files:
-            file_full_path = str(os.path.join(root, file))
-            inputs.append(file_full_path)
+        inputs.extend([str(os.path.join(root, file)) for file in files])
         if not recursive:
             break
 
@@ -101,12 +88,19 @@ def asset_snapshot(root_dir, manifest_out, recursive, **args):
         source_root = Path(asset_root_manifests.root_path)
         file_system_location_name = asset_root_manifests.file_system_location_name
         (_, _, manifest_name) = asset_uploader._gather_upload_metadata(
-            asset_root_manifests.asset_manifest, source_root, file_system_location_name
+            manifest=asset_root_manifests.asset_manifest,
+            source_root=source_root,
+            file_system_location_name=file_system_location_name,
         )
         asset_uploader._write_local_input_manifest(
-            manifest_out, manifest_name, asset_root_manifests.asset_manifest, root_dir_basename
+            manifest_write_dir=manifest_out,
+            manifest_name=manifest_name,
+            manifest=asset_root_manifests.asset_manifest,
+            root_dir_name=os.path.basename(root_dir),
         )
 
+    click.echo(f"Manifest created at {manifest_out}\n")
+
 
 @cli_asset.command(name="upload")
 @click.option(

src/deadline/job_attachments/upload.py

@@ -179,7 +179,7 @@ def upload_assets(
 
         if manifest_write_dir:
             self._write_local_manifest(
-                manifest_write_dir, manifest_name, full_manifest_key, manifest, None
+                manifest_write_dir, manifest_name, full_manifest_key, manifest
             )
 
         self.upload_bytes_to_s3(
@@ -224,7 +224,7 @@ def _write_local_manifest(
         manifest_name: str,
         full_manifest_key: str,
         manifest: BaseAssetManifest,
-        root_dir_name: Optional[str],
+        root_dir_name: Optional[str] = None,
     ) -> None:
         """
         Writes a manifest file locally in a 'manifests' sub-directory.
@@ -246,7 +246,7 @@ def _write_local_input_manifest(
         """
         input_manifest_folder_name = "manifests"
         if root_dir_name is not None:
-            input_manifest_folder_name = root_dir_name + input_manifest_folder_name
+            input_manifest_folder_name = root_dir_name + "_" + input_manifest_folder_name
 
         local_manifest_file = Path(manifest_write_dir, input_manifest_folder_name, manifest_name)
         logger.info(f"Creating local manifest file: {local_manifest_file}\n")

test/integ/cli/test_cli_asset.py

@@ -0,0 +1,201 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+
+"""
+Integ tests for the CLI asset commands.
+"""
+import os
+import json
+from click.testing import CliRunner
+import pytest
+import tempfile
+import posixpath
+from deadline.job_attachments.asset_manifests.hash_algorithms import hash_file, HashAlgorithm
+
+from deadline.client.cli import main
+
+
+TEST_FILE_CONTENT = "test file content"
+TEST_SUB_DIR_FILE_CONTENT = "subdir file content"
+TEST_ROOT_DIR_FILE_CONTENT = "root file content"
+
+TEST_ROOT_FILE = "root_file.txt"
+TEST_SUB_FILE = "subdir_file.txt"
+
+TEST_ROOT_DIR = "root_dir"
+TEST_SUB_DIR_1 = "subdir1"
+TEST_SUB_DIR_2 = "subdir2"
+
+
+class TestSnapshot:
+
+    @pytest.fixture
+    def temp_dir(self):
+        with tempfile.TemporaryDirectory() as tmpdir_path:
+            yield tmpdir_path
+
+    def test_root_dir_basic(self, temp_dir):
+        """
+        Snapshot with a valid root directory containing one file, and no other parameters
+        """
+        root_dir = os.path.join(temp_dir, TEST_ROOT_DIR)
+        os.makedirs(root_dir)
+        file_path = os.path.join(root_dir, TEST_ROOT_FILE)
+        with open(file_path, "w") as f:
+            f.write(TEST_FILE_CONTENT)
+
+        runner = CliRunner()
+        runner.invoke(main, ["asset", "snapshot", "--root-dir", root_dir])
+
+        # Check manifest file details to match correct content
+        # since manifest file name is hashed depending on source location, we have to list out manifest
+        manifest_folder_path = os.path.join(root_dir, f"{os.path.basename(root_dir)}_manifests")
+        manifest_files = os.listdir(manifest_folder_path)
+        assert (
+            len(manifest_files) == 1
+        ), f"Expected exactly one manifest file, but got {len(manifest_files)}"
+
+        manifest_file_name = manifest_files[0]
+        manifest_file_path = os.path.join(manifest_folder_path, manifest_file_name)
+
+        with open(manifest_file_path, "r") as f:
+            manifest_data = json.load(f)
+
+        expected_hash = hash_file(file_path, HashAlgorithm())  # 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_root_dir_not_recursive(self, temp_dir):
+        """
+        Snapshot with valid root directory with subdirectory and multiple files, but doesn't recursively snapshot.
+        """
+        root_dir = os.path.join(temp_dir, TEST_ROOT_DIR)
+
+        # Create a file in the root directory
+        root_file_path = os.path.join(root_dir, TEST_ROOT_FILE)
+        os.makedirs(os.path.dirname(root_file_path), exist_ok=True)
+        with open(root_file_path, "w") as f:
+            f.write(TEST_ROOT_DIR_FILE_CONTENT)
+
+        # Create a file in the subdirectory (should not be included)
+        subdir_file_path = os.path.join(root_dir, TEST_SUB_DIR_1, TEST_SUB_DIR_2, TEST_SUB_FILE)
+        os.makedirs(os.path.dirname(subdir_file_path), exist_ok=True)
+        with open(subdir_file_path, "w") as f:
+            f.write(TEST_SUB_DIR_FILE_CONTENT)
+
+        runner = CliRunner()
+        runner.invoke(main, ["asset", "snapshot", "--root-dir", root_dir])
+
+        # Check manifest file details to match correct content
+        manifest_folder_path = os.path.join(root_dir, f"{os.path.basename(root_dir)}_manifests")
+        manifest_files = os.listdir(manifest_folder_path)
+        assert (
+            len(manifest_files) == 1
+        ), f"Expected exactly one manifest file, but got {len(manifest_files)}"
+
+        manifest_file_name = manifest_files[0]
+        manifest_file_path = os.path.join(manifest_folder_path, manifest_file_name)
+
+        with open(manifest_file_path, "r") as f:
+            manifest_data = json.load(f)
+
+        # should ignore subdirectories
+        expected_hash = hash_file(root_file_path, HashAlgorithm())  # 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_root_dir_recursive(self, temp_dir):
+        """
+        Snapshot with valid root directory with subdirectory and multiple files, and recursively snapshots files.
+        """
+        root_dir = os.path.join(temp_dir, TEST_ROOT_DIR)
+
+        # Create a file in the root directory
+        root_file_path = os.path.join(root_dir, TEST_ROOT_DIR)
+        with open(root_file_path, "w") as f:
+            f.write(TEST_ROOT_DIR_FILE_CONTENT)
+
+        # Create a file in the subdirectory
+        subdir_file_path = os.path.join(root_dir, TEST_SUB_DIR_1, TEST_SUB_DIR_2, TEST_SUB_FILE)
+        os.makedirs(os.path.dirname(subdir_file_path), exist_ok=True)
+        with open(subdir_file_path, "w") as f:
+            f.write(TEST_SUB_DIR_FILE_CONTENT)
+
+        runner = CliRunner()
+        runner.invoke(main, ["asset", "snapshot", "--root-dir", root_dir, "--recursive"])
+
+        # Check manifest file details to match correct content
+        # since manifest file name is hashed depending on source location, we have to list out manifest
+        manifest_folder_path = os.path.join(root_dir, f"{os.path.basename(root_dir)}_manifests")
+        manifest_files = os.listdir(manifest_folder_path)
+        assert (
+            len(manifest_files) == 1
+        ), f"Expected exactly one manifest file, but got {len(manifest_files)}"
+
+        root_manifest_file_name = [file for file in manifest_files][0]
+        root_manifest_file_path = os.path.join(manifest_folder_path, root_manifest_file_name)
+
+        with open(root_manifest_file_path, "r") as f:
+            manifest_data = json.load(f)
+
+        root_file_hash = hash_file(root_file_path, HashAlgorithm())  # hashed with xxh128
+        subdir_file_hash = hash_file(subdir_file_path, HashAlgorithm())  # hashed with xxh128
+        manifest_data_paths = manifest_data["paths"]
+        assert (
+            len(manifest_data_paths) == 2
+        ), f"Expected exactly 2 paths inside manifest, but got {len(manifest_data_paths)}"
+        assert manifest_data_paths[0]["path"] == TEST_ROOT_FILE
+        assert manifest_data_paths[0]["hash"] == root_file_hash
+        assert manifest_data_paths[1]["path"] == posixpath.join(
+            TEST_SUB_DIR_1, TEST_SUB_DIR_2, TEST_SUB_FILE
+        )
+        assert manifest_data_paths[1]["hash"] == subdir_file_hash
+
+    def test_specified_manifest_out(self, temp_dir):
+        """
+        Snapshot with valid root directory, checks if manifest is created in the specified --manifest-out location
+        """
+        root_dir = os.path.join(temp_dir, TEST_ROOT_DIR)
+        os.makedirs(root_dir)
+        manifest_out_dir = os.path.join(temp_dir, "manifest_out")
+        os.makedirs(manifest_out_dir)
+        file_path = os.path.join(root_dir, TEST_ROOT_FILE)
+        with open(file_path, "w") as f:
+            f.write(TEST_FILE_CONTENT)
+
+        runner = CliRunner()
+        runner.invoke(
+            main, ["asset", "snapshot", "--root-dir", root_dir, "--manifest-out", manifest_out_dir]
+        )
+
+        # Check manifest file details to match correct content
+        # since manifest file name is hashed depending on source location, we have to list out manifest
+        manifest_folder_path = os.path.join(
+            manifest_out_dir, f"{os.path.basename(root_dir)}_manifests"
+        )
+        manifest_files = os.listdir(manifest_folder_path)
+        assert (
+            len(manifest_files) == 1
+        ), f"Expected exactly one manifest file, but got {len(manifest_files)}"
+
+        manifest_file_name = manifest_files[0]
+
+        manifest_file_path = os.path.join(manifest_folder_path, manifest_file_name)
+
+        with open(manifest_file_path, "r") as f:
+            manifest_data = json.load(f)
+
+        expected_hash = hash_file(file_path, HashAlgorithm())  # 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