From 6420e528e3a113da489adba2dea0edfab0fb242f Mon Sep 17 00:00:00 2001 From: Cor Date: Tue, 2 Jul 2024 20:03:58 +0200 Subject: [PATCH] Handle missing permission to read file (#1949) Handle missing permissions when reading files during linting ### Linked issues Resolves #1942 Partially resolves #1952 --------- Co-authored-by: Andrew Snare --- .../labs/ucx/source_code/notebooks/loaders.py | 8 +++++- .../labs/ucx/source_code/notebooks/sources.py | 13 ++++++--- .../unit/source_code/notebooks/test_loader.py | 22 ++++++++++++++- .../source_code/notebooks/test_sources.py | 27 +++++++++++++++++++ 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/databricks/labs/ucx/source_code/notebooks/loaders.py b/src/databricks/labs/ucx/source_code/notebooks/loaders.py index 825fa46108..764469c47c 100644 --- a/src/databricks/labs/ucx/source_code/notebooks/loaders.py +++ b/src/databricks/labs/ucx/source_code/notebooks/loaders.py @@ -53,7 +53,13 @@ def load_dependency(self, path_lookup: PathLookup, dependency: Dependency) -> So try: content = absolute_path.read_text("utf-8") except NotFound: - logger.warning(f"Could not read notebook from workspace: {absolute_path}") + logger.warning(f"Path not found trying to read notebook from workspace: {absolute_path}") + return None + except PermissionError: + logger.warning( + f"Permission error while reading notebook from workspace: {absolute_path}", + exc_info=True, + ) return None language = self._detect_language(absolute_path, content) if not language: diff --git a/src/databricks/labs/ucx/source_code/notebooks/sources.py b/src/databricks/labs/ucx/source_code/notebooks/sources.py index babf677d2b..42177876af 100644 --- a/src/databricks/labs/ucx/source_code/notebooks/sources.py +++ b/src/databricks/labs/ucx/source_code/notebooks/sources.py @@ -198,10 +198,18 @@ def lint(self) -> Iterable[Advice]: encoding = locale.getpreferredencoding(False) try: is_notebook = self._is_notebook() + except FileNotFoundError: + failure_message = f"File not found: {self._path}" + yield Failure("file-not-found", failure_message, 0, 0, 1, 1) + return except UnicodeDecodeError: failure_message = f"File without {encoding} encoding is not supported {self._path}" yield Failure("unsupported-file-encoding", failure_message, 0, 0, 1, 1) return + except PermissionError: + failure_message = f"Missing read permission for {self._path}" + yield Failure("file-permission", failure_message, 0, 0, 1, 1) + return if is_notebook: yield from self._lint_notebook() @@ -223,9 +231,8 @@ def _lint_file(self): linter = self._ctx.linter(language) yield from linter.lint(self._source_code) except ValueError as err: - yield Failure( - "unsupported-content", f"Error while parsing content of {self._path.as_posix()}: {err}", 0, 0, 1, 1 - ) + failure_message = f"Error while parsing content of {self._path.as_posix()}: {err}" + yield Failure("unsupported-content", failure_message, 0, 0, 1, 1) def _lint_notebook(self): notebook = Notebook.parse(self._path, self._source_code, self._file_language()) diff --git a/tests/unit/source_code/notebooks/test_loader.py b/tests/unit/source_code/notebooks/test_loader.py index ebcd7e4bc2..a87b2e3312 100644 --- a/tests/unit/source_code/notebooks/test_loader.py +++ b/tests/unit/source_code/notebooks/test_loader.py @@ -1,8 +1,13 @@ +import logging from pathlib import Path +from unittest.mock import create_autospec -from databricks.labs.ucx.source_code.notebooks.loaders import NotebookLoader from databricks.sdk.service.workspace import Language +from databricks.labs.ucx.source_code.graph import Dependency +from databricks.labs.ucx.source_code.notebooks.loaders import NotebookLoader +from databricks.labs.ucx.source_code.path_lookup import PathLookup + def test_detects_language(): @@ -17,3 +22,18 @@ def detect_language(cls, path: Path, content: str): assert NotebookLoaderForTesting.detect_language(Path("hi"), "# Databricks notebook source") == Language.PYTHON assert NotebookLoaderForTesting.detect_language(Path("hi"), "-- Databricks notebook source") == Language.SQL assert not NotebookLoaderForTesting.detect_language(Path("hi"), "stuff") + + +def test_notebook_loader_loads_dependency_with_permission_error(caplog): + path = create_autospec(Path) + path.read_text.side_effect = PermissionError("Permission denied") + path_lookup = create_autospec(PathLookup) + path_lookup.resolve.return_value = path + dependency = create_autospec(Dependency) + dependency.path.return_value = path + + with caplog.at_level(logging.WARNING, logger="databricks.labs.ucx.source_code.notebooks.loaders"): + found = NotebookLoader().load_dependency(path_lookup, dependency) + + assert f"Permission error while reading notebook from workspace: {path}" in caplog.text + assert found is None diff --git a/tests/unit/source_code/notebooks/test_sources.py b/tests/unit/source_code/notebooks/test_sources.py index 31bcb0bbfb..390737f5ab 100644 --- a/tests/unit/source_code/notebooks/test_sources.py +++ b/tests/unit/source_code/notebooks/test_sources.py @@ -1,6 +1,7 @@ import codecs import locale from pathlib import Path +from unittest.mock import create_autospec import pytest @@ -85,3 +86,29 @@ def test_file_linter_lints_non_ascii_encoded_file(migration_index): assert len(advices) == 1 assert advices[0].code == "unsupported-file-encoding" assert advices[0].message == f"File without {preferred_encoding} encoding is not supported {non_ascii_encoded_file}" + + +def test_file_linter_lints_file_with_missing_file(migration_index): + path = create_autospec(Path) + path.suffix = ".py" + path.read_text.side_effect = FileNotFoundError("No such file or directory: 'test.py'") + linter = FileLinter(LinterContext(migration_index), path) + + advices = list(linter.lint()) + + assert len(advices) == 1 + assert advices[0].code == "file-not-found" + assert advices[0].message == f"File not found: {path}" + + +def test_file_linter_lints_file_with_missing_read_permission(migration_index): + path = create_autospec(Path) + path.suffix = ".py" + path.read_text.side_effect = PermissionError("Permission denied") + linter = FileLinter(LinterContext(migration_index), path) + + advices = list(linter.lint()) + + assert len(advices) == 1 + assert advices[0].code == "file-permission" + assert advices[0].message == f"Missing read permission for {path}"