diff --git a/src/deadline/client/api/_submit_job_bundle.py b/src/deadline/client/api/_submit_job_bundle.py index a93ea315..17a1ed30 100644 --- a/src/deadline/client/api/_submit_job_bundle.py +++ b/src/deadline/client/api/_submit_job_bundle.py @@ -32,6 +32,7 @@ JobParameter, ) from ..job_bundle.submission import AssetReferences, split_parameter_args +from ...job_attachments.exceptions import MisconfiguredInputsError from ...job_attachments.models import ( JobAttachmentsFileSystem, AssetRootGroup, @@ -210,13 +211,46 @@ def create_job_from_job_bundle( # Hash and upload job attachments if there are any if asset_references and "jobAttachmentSettings" in queue: # Extend input_filenames with all the files in the input_directories + missing_directories: set[str] = set() + empty_directories: set[str] = set() for directory in asset_references.input_directories: + if not os.path.isdir(directory): + missing_directories.add(directory) + continue + + is_dir_empty = True for root, _, files in os.walk(directory): + if not files: + continue + is_dir_empty = False asset_references.input_filenames.update( os.path.normpath(os.path.join(root, file)) for file in files ) + if is_dir_empty: + empty_directories.add(directory) asset_references.input_directories.clear() + misconfigured_directories = missing_directories or empty_directories + if misconfigured_directories: + all_misconfigured_inputs = "" + misconfigured_directories_msg = ( + "Job submission contains misconfigured input directories and cannot be submitted." + " All input directories must exist and cannot be empty." + ) + + if missing_directories: + missing_directory_list = sorted(list(missing_directories)) + all_missing_directories = "\n\t".join(missing_directory_list) + all_misconfigured_inputs += ( + f"\nNon-existent directories:\n\t{all_missing_directories}" + ) + if empty_directories: + empty_directory_list = sorted(list(empty_directories)) + all_empty_directories = "\n\t".join(empty_directory_list) + all_misconfigured_inputs += f"\nEmpty directories:\n\t{all_empty_directories}" + + raise MisconfiguredInputsError(misconfigured_directories_msg + all_misconfigured_inputs) + queue_role_session = api.get_queue_user_boto3_session( deadline=deadline, config=config, diff --git a/src/deadline/client/cli/_groups/bundle_group.py b/src/deadline/client/cli/_groups/bundle_group.py index a729e8b7..0ec8b383 100644 --- a/src/deadline/client/cli/_groups/bundle_group.py +++ b/src/deadline/client/cli/_groups/bundle_group.py @@ -13,8 +13,12 @@ from botocore.exceptions import ClientError from deadline.client import api -from deadline.client.config import config_file, get_setting, set_setting -from deadline.job_attachments.exceptions import AssetSyncError, AssetSyncCancelledError +from deadline.client.config import set_setting +from deadline.job_attachments.exceptions import ( + AssetSyncError, + AssetSyncCancelledError, + MisconfiguredInputsError, +) from deadline.job_attachments.models import JobAttachmentsFileSystem from deadline.job_attachments.progress_tracker import ProgressReportMetadata from deadline.job_attachments._utils import _human_readable_file_size @@ -124,18 +128,19 @@ def _decide_cancel_submission( if deviated_file_count_by_root: root_by_count_message = "\n\n".join( [ - f"{file_count} files from : '{directory}'" + f"{file_count} files from: '{directory}'" for directory, file_count in deviated_file_count_by_root.items() ] ) message_text += ( f"\n\nFiles were found outside of the configured storage profile location(s). " " Please confirm that you intend to upload files from the following directories:\n\n" - f"{root_by_count_message}" + f"{root_by_count_message}\n\n" + "To permanently remove this warning you must only upload files located within a storage profile location." ) message_text += "\n\nDo you wish to proceed?" return ( - not (yes or config_file.str2bool(get_setting("settings.auto_accept", config=config))) + not yes and num_files > 0 and not click.confirm( message_text, @@ -191,6 +196,10 @@ def _decide_cancel_submission( raise DeadlineOperationError( f"Failed to submit the job bundle to AWS Deadline Cloud:\n{exc}" ) from exc + except MisconfiguredInputsError as exc: + click.echo(str(exc)) + click.echo("Job submission canceled.") + return except Exception as exc: api.get_deadline_cloud_library_telemetry_client().record_error( event_details={"exception_scope": "on_submit"}, diff --git a/src/deadline/client/ui/dialogs/deadline_config_dialog.py b/src/deadline/client/ui/dialogs/deadline_config_dialog.py index fe7cc465..a76c3a0d 100644 --- a/src/deadline/client/ui/dialogs/deadline_config_dialog.py +++ b/src/deadline/client/ui/dialogs/deadline_config_dialog.py @@ -280,7 +280,7 @@ def _build_farm_settings_ui(self, group, layout): def _build_general_settings_ui(self, group, layout): self.auto_accept = self._init_checkbox_setting( - group, layout, "settings.auto_accept", "Auto Accept Confirmation Prompts" + group, layout, "settings.auto_accept", "Auto Accept Prompt Defaults" ) self.telemetry_opt_out = self._init_checkbox_setting( group, layout, "telemetry.opt_out", "Telemetry Opt Out" diff --git a/src/deadline/client/ui/dialogs/submit_job_progress_dialog.py b/src/deadline/client/ui/dialogs/submit_job_progress_dialog.py index a4049b2d..b2fc4f33 100644 --- a/src/deadline/client/ui/dialogs/submit_job_progress_dialog.py +++ b/src/deadline/client/ui/dialogs/submit_job_progress_dialog.py @@ -24,7 +24,6 @@ QLabel, QMessageBox, QProgressBar, - QPushButton, QTextEdit, QVBoxLayout, QWidget, @@ -52,7 +51,7 @@ AssetReferences, split_parameter_args, ) -from deadline.job_attachments.exceptions import AssetSyncCancelledError +from deadline.job_attachments.exceptions import AssetSyncCancelledError, MisconfiguredInputsError from deadline.job_attachments.models import AssetRootGroup, AssetRootManifest, StorageProfile from deadline.job_attachments.progress_tracker import ProgressReportMetadata, SummaryStatistics from deadline.job_attachments.upload import S3AssetManager @@ -240,13 +239,68 @@ def _start_submission(self): or self.asset_references.output_directories ): # Extend input_filenames with all the files in the input_directories + missing_directories: set[str] = set() + empty_directories: set[str] = set() for directory in self.asset_references.input_directories: + if not os.path.isdir(directory): + missing_directories.add(directory) + continue + + is_dir_empty = True for root, _, files in os.walk(directory): + if not files: + continue + is_dir_empty = False self.asset_references.input_filenames.update( os.path.normpath(os.path.join(root, file)) for file in files ) + if is_dir_empty: + empty_directories.add(directory) self.asset_references.input_directories.clear() + misconfigured_directories = missing_directories or empty_directories + if misconfigured_directories: + sample_size = 3 + sample_of_misconfigured_inputs = "" + all_misconfigured_inputs = "" + misconfigured_directories_msg = ( + "Job submission contains misconfigured input directories and cannot be submitted." + " All input directories must exist and cannot be empty." + ) + + if missing_directories: + missing_directory_list = sorted(list(missing_directories)) + sample_of_missing_directories = "\n\t".join( + missing_directory_list[:sample_size] + ) + sample_of_misconfigured_inputs += ( + f"\nNon-existent directories:\n\t{sample_of_missing_directories}\n" + ) + all_missing_directories = "\n\t".join(missing_directory_list) + all_misconfigured_inputs += ( + f"\nNon-existent directories:\n\t{all_missing_directories}" + ) + if empty_directories: + empty_directory_list = sorted(list(empty_directories)) + sample_of_empty_directories = "\n\t".join(empty_directory_list[:sample_size]) + sample_of_misconfigured_inputs += ( + f"\nEmpty directories:\n\t{sample_of_empty_directories}" + ) + all_empty_directories = "\n\t".join(empty_directory_list) + all_misconfigured_inputs += f"\nEmpty directories:\n\t{all_empty_directories}" + + logging.error(misconfigured_directories_msg + all_misconfigured_inputs) + just_a_sample = ( + len(missing_directories) > sample_size or len(empty_directories) > sample_size + ) + if just_a_sample: + misconfigured_directories_msg += ( + " Check logs for all occurrences, here's a sample:\n" + ) + misconfigured_directories_msg += f"{sample_of_misconfigured_inputs}" + + raise MisconfiguredInputsError(misconfigured_directories_msg) + upload_group = self._asset_manager.prepare_paths_for_upload( job_bundle_path=self._job_bundle_dir, input_paths=sorted(self.asset_references.input_filenames), @@ -256,13 +310,10 @@ def _start_submission(self): ) # If we find any Job Attachments, start a background thread if upload_group.asset_groups: - if ( - not self._auto_accept - and not self._confirm_asset_references_outside_storage_profile( - upload_group.num_outside_files_by_root, - upload_group.total_input_files, - upload_group.total_input_bytes, - ) + if not self._confirm_asset_references_outside_storage_profile( + upload_group.num_outside_files_by_root, + upload_group.total_input_files, + upload_group.total_input_bytes, ): raise UserInitiatedCancel("Submission canceled.") @@ -561,26 +612,21 @@ def _confirm_asset_references_outside_storage_profile( if deviated_file_count_by_root: root_by_count_message = "\n\n".join( [ - f"{file_count} files from : '{directory}'" + f"{file_count} files from: '{directory}'" for directory, file_count in deviated_file_count_by_root.items() ] ) message_text += ( f"\n\nFiles were found outside of the configured storage profile location(s). " " Please confirm that you intend to upload files from the following directories:\n\n" - f"{root_by_count_message}" + f"{root_by_count_message}\n\n" + "To remove this warning you must only upload files located within a storage profile location." ) message_box.setIcon(QMessageBox.Warning) message_box.setText(message_text) message_box.setStandardButtons(QMessageBox.Ok | QMessageBox.Cancel) message_box.setDefaultButton(QMessageBox.Ok) - # Add the "Do not ask again" button that acts like 'OK' but sets the config - # setting to always auto-accept similar prompts in the future. - dont_ask_button = QPushButton("Do not ask again", self) - dont_ask_button.clicked.connect(lambda: set_setting("settings.auto_accept", "true")) - message_box.addButton(dont_ask_button, QMessageBox.ActionRole) - message_box.setWindowTitle("Job Attachments Valid Files Confirmation") selection = message_box.exec() diff --git a/src/deadline/job_attachments/exceptions.py b/src/deadline/job_attachments/exceptions.py index 829f1301..3d69e967 100644 --- a/src/deadline/job_attachments/exceptions.py +++ b/src/deadline/job_attachments/exceptions.py @@ -92,6 +92,13 @@ class AssetOutsideOfRootError(JobAttachmentsError): """ +class MisconfiguredInputsError(JobAttachmentsError): + """ + Exception for errors related to missing input directories, empty input directories, + missing input files, and input directories classified as files + """ + + class ManifestDecodeValidationError(JobAttachmentsError): """ Exception for errors related to asset manifest decoding. diff --git a/src/deadline/job_attachments/upload.py b/src/deadline/job_attachments/upload.py index b2872b22..e71d1ab4 100644 --- a/src/deadline/job_attachments/upload.py +++ b/src/deadline/job_attachments/upload.py @@ -46,6 +46,7 @@ AssetSyncError, JobAttachmentS3BotoCoreError, JobAttachmentsS3ClientError, + MisconfiguredInputsError, MissingS3BucketError, MissingS3RootPrefixError, ) @@ -838,17 +839,18 @@ def _get_asset_groups( relative to one of the AssetRootGroup objects returned. """ groupings: dict[str, AssetRootGroup] = {} + missing_input_paths = set() + misconfigured_directories = set() # Resolve full path, then cast to pure path to get top-level directory - # Note for inputs, we only upload individual files so user doesn't unintentionally upload the entire hard drive for _path in input_paths: # Need to use absolute to not resolve symlinks, but need normpath to get rid of relative paths, i.e. '..' abs_path = Path(os.path.normpath(Path(_path).absolute())) if not abs_path.exists(): - logger.warning(f"Skipping uploading input as it doesn't exist: {abs_path}") + missing_input_paths.add(abs_path) continue if abs_path.is_dir(): - logger.warning(f"Skipping uploading input as it is a directory: {abs_path}") + misconfigured_directories.add(abs_path) continue # Skips the upload if the path is relative to any of the File System Location @@ -866,6 +868,26 @@ def _get_asset_groups( matched_group = self._get_matched_group(matched_root, groupings) matched_group.inputs.add(abs_path) + if missing_input_paths or misconfigured_directories: + all_misconfigured_inputs = "" + misconfigured_inputs_msg = ( + "Job submission contains missing input files or directories specified as files." + " All inputs must exist and be classified properly." + ) + if missing_input_paths: + missing_inputs_list: list[str] = sorted([str(i) for i in missing_input_paths]) + all_missing_inputs = "\n\t".join(missing_inputs_list) + all_misconfigured_inputs += f"\nMissing input files:\n\t{all_missing_inputs}" + if misconfigured_directories: + misconfigured_directories_list: list[str] = sorted( + [str(d) for d in misconfigured_directories] + ) + all_misconfigured_directories = "\n\t".join(misconfigured_directories_list) + all_misconfigured_inputs += ( + f"\nDirectories classified as files:\n\t{all_misconfigured_directories}" + ) + raise MisconfiguredInputsError(misconfigured_inputs_msg + all_misconfigured_inputs) + for _path in output_paths: abs_path = Path(os.path.normpath(Path(_path).absolute())) diff --git a/test/unit/deadline_client/api/test_job_bundle_submission.py b/test/unit/deadline_client/api/test_job_bundle_submission.py index db4e1dea..bd095be1 100644 --- a/test/unit/deadline_client/api/test_job_bundle_submission.py +++ b/test/unit/deadline_client/api/test_job_bundle_submission.py @@ -6,6 +6,7 @@ import json import os +from pathlib import Path from typing import Any, Dict, Tuple from unittest.mock import ANY, patch, Mock from deadline.client import exceptions @@ -15,6 +16,7 @@ from deadline.client import api, config from deadline.client.api import _submit_job_bundle +from deadline.job_attachments.exceptions import MisconfiguredInputsError from deadline.job_attachments.models import ( AssetRootGroup, AssetUploadGroup, @@ -747,6 +749,154 @@ def test_create_job_from_job_bundle_with_empty_asset_references( ) +def test_create_job_from_job_bundle_partially_empty_directories( + fresh_deadline_config, temp_job_bundle_dir +): + """ + Test a job bundle with an input directory that contains both empty directories and input files + does not throw a MisconfiguredInputsError and successfully submits + """ + job_template_type, job_template = MOCK_JOB_TEMPLATE_CASES["MINIMAL_JSON"] + temp_bundle_dir_as_path = Path(temp_job_bundle_dir) + assets_directory: str = str(temp_bundle_dir_as_path / "assets") + empty_directory = str(temp_bundle_dir_as_path / "assets" / "empty_dir") + Path(empty_directory).mkdir(parents=True) + (temp_bundle_dir_as_path / "assets" / "input_file").touch() + + with patch.object(_submit_job_bundle.api, "get_boto3_client") as client_mock, patch.object( + _submit_job_bundle.api, "get_queue_user_boto3_session" + ): + client_mock().create_job.side_effect = [MOCK_CREATE_JOB_RESPONSE] + client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] + config.set_setting("defaults.farm_id", MOCK_FARM_ID) + config.set_setting("defaults.queue_id", MOCK_QUEUE_ID) + + # Write the template to the job bundle + with open( + os.path.join(temp_job_bundle_dir, f"template.{job_template_type.lower()}"), + "w", + encoding="utf8", + ) as f: + f.write(job_template) + + # Write an asset_references.json + asset_references: dict = { + "inputs": {"filenames": [], "directories": [assets_directory]}, + "outputs": {"directories": []}, + } + with open( + os.path.join(temp_job_bundle_dir, "asset_references.json"), "w", encoding="utf8" + ) as f: + json.dump({"assetReferences": asset_references}, f) + + # WHEN + response = api.create_job_from_job_bundle( + job_bundle_dir=temp_job_bundle_dir, + queue_parameter_definitions=[], + ) + + # THEN + # create_job_from_job_bundle did NOT throw MisconfiguredInputsError + assert response == MOCK_JOB_ID + + +def test_create_job_from_job_bundle_misconfigured_directories( + fresh_deadline_config, temp_job_bundle_dir +): + """ + Test a job bundle with input directories that do not exist, or are empty + """ + job_template_type, job_template = MOCK_JOB_TEMPLATE_CASES["MINIMAL_JSON"] + temp_bundle_dir_as_path = Path(temp_job_bundle_dir) + missing_directory = str(temp_bundle_dir_as_path / "does" / "not" / "exist") + empty_directory = str(temp_bundle_dir_as_path / "empty_dir") + Path(empty_directory).mkdir() + + with patch.object(api._session, "get_boto3_session"), patch.object( + _submit_job_bundle.api, "get_boto3_client" + ) as client_mock: + client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] + config.set_setting("defaults.farm_id", MOCK_FARM_ID) + config.set_setting("defaults.queue_id", MOCK_QUEUE_ID) + + # Write the template to the job bundle + with open( + os.path.join(temp_job_bundle_dir, f"template.{job_template_type.lower()}"), + "w", + encoding="utf8", + ) as f: + f.write(job_template) + + # Write an asset_references.json + asset_references: dict = { + "inputs": {"filenames": [], "directories": [missing_directory, empty_directory]}, + "outputs": {"directories": []}, + } + with open( + os.path.join(temp_job_bundle_dir, "asset_references.json"), "w", encoding="utf8" + ) as f: + json.dump({"assetReferences": asset_references}, f) + + # WHEN / THEN + with pytest.raises( + MisconfiguredInputsError, match=f"{missing_directory}|{empty_directory}" + ): + api.create_job_from_job_bundle( + job_bundle_dir=temp_job_bundle_dir, + queue_parameter_definitions=[], + ) + + +def test_create_job_from_job_bundle_misconfigured_input_files( + fresh_deadline_config, temp_job_bundle_dir +): + """ + Test a job bundle with input files that do not exist, or are actually a directory + """ + job_template_type, job_template = MOCK_JOB_TEMPLATE_CASES["MINIMAL_JSON"] + temp_bundle_dir_as_path = Path(temp_job_bundle_dir) + missing_file = str(temp_bundle_dir_as_path / "does" / "not" / "exist.png") + directory_pretending_to_be_file = str(temp_bundle_dir_as_path / "not a file") + Path(directory_pretending_to_be_file).mkdir() + + with patch.object(api._session, "get_boto3_session"), patch.object( + _submit_job_bundle.api, "get_boto3_client" + ) as client_mock, patch.object(_submit_job_bundle.api, "get_queue_user_boto3_session"): + client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] + config.set_setting("defaults.farm_id", MOCK_FARM_ID) + config.set_setting("defaults.queue_id", MOCK_QUEUE_ID) + + # Write the template to the job bundle + with open( + os.path.join(temp_job_bundle_dir, f"template.{job_template_type.lower()}"), + "w", + encoding="utf8", + ) as f: + f.write(job_template) + + # Write an asset_references.json + asset_references: dict = { + "inputs": { + "filenames": [missing_file, directory_pretending_to_be_file], + "directories": [], + }, + "outputs": {"directories": []}, + } + with open( + os.path.join(temp_job_bundle_dir, "asset_references.json"), "w", encoding="utf8" + ) as f: + json.dump({"assetReferences": asset_references}, f) + + # WHEN / THEN + with pytest.raises( + MisconfiguredInputsError, match=rf"{missing_file}|{directory_pretending_to_be_file}" + ): + api.create_job_from_job_bundle( + job_bundle_dir=temp_job_bundle_dir, + queue_parameter_definitions=[], + ) + + def test_create_job_from_job_bundle_with_single_asset_file( fresh_deadline_config, temp_job_bundle_dir, temp_assets_dir ): diff --git a/test/unit/deadline_job_attachments/test_upload.py b/test/unit/deadline_job_attachments/test_upload.py index d203f6f6..3e46569d 100644 --- a/test/unit/deadline_job_attachments/test_upload.py +++ b/test/unit/deadline_job_attachments/test_upload.py @@ -9,7 +9,7 @@ from copy import deepcopy from datetime import datetime from io import BytesIO -from logging import DEBUG, WARNING +from logging import DEBUG from pathlib import Path from typing import Dict, List, Set, Tuple from unittest.mock import MagicMock, patch @@ -33,6 +33,7 @@ from deadline.job_attachments.exceptions import ( AssetSyncError, JobAttachmentsS3ClientError, + MisconfiguredInputsError, MissingS3BucketError, MissingS3RootPrefixError, ) @@ -1657,17 +1658,18 @@ def test_process_input_path_skip_file_already_in_hash_cache(self, farm_id, queue @mock_sts def test_asset_management_input_not_exists(self, farm_id, queue_id, tmpdir, caplog): - """Test the input paths that does not exist are properly skipped""" + """Ensure that when: + * input paths do not exist, or + * directories are classified as files + the submission is prevented with a MisconfiguredInputsError + """ asset_root = str(tmpdir) # GIVEN scene_file = tmpdir.mkdir("scene").join("maya.ma") scene_file.write("a") input_not_exist = "/texture/that/doesnt/exist.anywhere" - - cache_dir = tmpdir.mkdir("cache") - - expected_total_input_bytes = scene_file.size() + directory_as_file = str(Path(scene_file).parent) with patch( f"{deadline.__package__}.job_attachments.upload.PathFormat.get_host_path_format", @@ -1678,11 +1680,6 @@ def test_asset_management_input_not_exists(self, farm_id, queue_id, tmpdir, capl ), patch( f"{deadline.__package__}.job_attachments.upload.hash_file", side_effect=["a"] ): - caplog.set_level(WARNING) - - mock_on_preparing_to_submit = MagicMock(return_value=True) - mock_on_uploading_assets = MagicMock(return_value=True) - asset_manager = S3AssetManager( farm_id=farm_id, queue_id=queue_id, @@ -1690,55 +1687,16 @@ def test_asset_management_input_not_exists(self, farm_id, queue_id, tmpdir, capl asset_manifest_version=ManifestVersion.v2023_03_03, ) - # When - upload_group = asset_manager.prepare_paths_for_upload( - job_bundle_path=str(asset_root), - input_paths=[input_not_exist, scene_file], - output_paths=[str(Path(asset_root).joinpath("outputs"))], - referenced_paths=[], - ) - ( - hash_summary_statistics, - asset_root_manifests, - ) = asset_manager.hash_assets_and_create_manifest( - asset_groups=upload_group.asset_groups, - total_input_files=upload_group.total_input_files, - total_input_bytes=upload_group.total_input_bytes, - hash_cache_dir=cache_dir, - on_preparing_to_submit=mock_on_preparing_to_submit, - ) - - (upload_summary_statistics, _) = asset_manager.upload_assets( - manifests=asset_root_manifests, - on_uploading_assets=mock_on_uploading_assets, - s3_check_cache_dir=cache_dir, - ) - - # Then - assert "Skipping uploading input as it doesn't exist: " in caplog.text - - assert_progress_report_last_callback( - num_input_files=1, - expected_total_input_bytes=expected_total_input_bytes, - on_preparing_to_submit=mock_on_preparing_to_submit, - on_uploading_assets=mock_on_uploading_assets, - ) - - assert_progress_report_summary_statistics( - actual_summary_statistics=hash_summary_statistics, - processed_files=1, - processed_bytes=expected_total_input_bytes, - skipped_files=0, - skipped_bytes=0, - ) - - assert_progress_report_summary_statistics( - actual_summary_statistics=upload_summary_statistics, - processed_files=1, - processed_bytes=expected_total_input_bytes, - skipped_files=0, - skipped_bytes=0, - ) + # WHEN / THEN + with pytest.raises( + MisconfiguredInputsError, match=f"{input_not_exist}|{directory_as_file}" + ): + asset_manager.prepare_paths_for_upload( + job_bundle_path=str(asset_root), + input_paths=[input_not_exist, directory_as_file, scene_file], + output_paths=[str(Path(asset_root).joinpath("outputs"))], + referenced_paths=[], + ) @mock_sts @pytest.mark.parametrize(