diff --git a/samcli/lib/utils/definition_validator.py b/samcli/lib/utils/definition_validator.py index bab018a632..2a92daf47d 100644 --- a/samcli/lib/utils/definition_validator.py +++ b/samcli/lib/utils/definition_validator.py @@ -1,6 +1,7 @@ """DefinitionValidator for Validating YAML and JSON Files""" import logging +import time from pathlib import Path from typing import Any, Dict, Optional @@ -11,6 +12,9 @@ LOG = logging.getLogger(__name__) +FILE_READ_ATTEMPTS = 3 +FILE_READ_RETRY_DELAY = 0.1 + class DefinitionValidator: _path: Path @@ -75,7 +79,7 @@ def validate_file(self) -> bool: return False try: - self._data = parse_yaml_file(str(self._path)) + self._data = self._parse_file() except (ValueError, yaml.YAMLError) as e: LOG.debug( "File %s failed to validate due to it file cannot be parsed. \ @@ -84,4 +88,32 @@ def validate_file(self) -> bool: exc_info=e, ) return False + except OSError as e: + LOG.warning( + "File %s failed to validate because it cannot be read. \ +The change will not be synced until the file is saved again.", + self._path, + exc_info=e, + ) + return False return True + + def _parse_file(self) -> Dict[str, Any]: + """Read and parse the definition file, retrying while it is locked by another process. + + Returns + ------- + Dict[str, Any] + Parsed content of the definition file. + """ + remaining_attempts = FILE_READ_ATTEMPTS + delay = FILE_READ_RETRY_DELAY + while True: + try: + return parse_yaml_file(str(self._path)) + except PermissionError: + remaining_attempts -= 1 + if not remaining_attempts: + raise + time.sleep(delay) + delay *= 2 diff --git a/tests/unit/lib/utils/test_definition_validator.py b/tests/unit/lib/utils/test_definition_validator.py index bbb16e1a4f..4a203ec8c8 100644 --- a/tests/unit/lib/utils/test_definition_validator.py +++ b/tests/unit/lib/utils/test_definition_validator.py @@ -69,3 +69,29 @@ def test_detect_change_for_file_opened_event(self, parse_yaml_file_mock): validator = DefinitionValidator(self.path, detect_change=True, initialize_data=True) event = FileOpenedEvent("src_path") self.assertFalse(validator.validate_change(event)) + + @patch("samcli.lib.utils.definition_validator.time.sleep") + @patch("samcli.lib.utils.definition_validator.parse_yaml_file") + def test_detect_change_retries_locked_file(self, parse_yaml_file_mock, sleep_mock): + parse_yaml_file_mock.side_effect = [{"A": 1}, PermissionError(13, "Permission denied"), {"B": 1}] + + validator = DefinitionValidator(self.path, detect_change=True, initialize_data=True) + self.assertTrue(validator.validate_change()) + + @patch("samcli.lib.utils.definition_validator.time.sleep") + @patch("samcli.lib.utils.definition_validator.parse_yaml_file") + def test_detect_change_locked_file(self, parse_yaml_file_mock, sleep_mock): + parse_yaml_file_mock.side_effect = PermissionError(13, "Permission denied") + + validator = DefinitionValidator(self.path, detect_change=True, initialize_data=False) + self.assertFalse(validator.validate_change()) + self.assertEqual(parse_yaml_file_mock.call_count, 3) + + @patch("samcli.lib.utils.definition_validator.time.sleep") + @patch("samcli.lib.utils.definition_validator.parse_yaml_file") + def test_detect_change_unreadable_file(self, parse_yaml_file_mock, sleep_mock): + parse_yaml_file_mock.side_effect = IsADirectoryError(21, "Is a directory") + + validator = DefinitionValidator(self.path, detect_change=True, initialize_data=False) + self.assertFalse(validator.validate_change()) + self.assertEqual(parse_yaml_file_mock.call_count, 1)