Skip to content

Commit

Permalink
Handle missing permission to read file (#1949)
Browse files Browse the repository at this point in the history
Handle missing permissions when reading files during linting

### Linked issues
Resolves #1942
Partially resolves #1952

---------

Co-authored-by: Andrew Snare <[email protected]>
  • Loading branch information
JCZuurmond and asnare authored Jul 2, 2024
1 parent d074387 commit 6420e52
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 5 deletions.
8 changes: 7 additions & 1 deletion src/databricks/labs/ucx/source_code/notebooks/loaders.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 10 additions & 3 deletions src/databricks/labs/ucx/source_code/notebooks/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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())
Expand Down
22 changes: 21 additions & 1 deletion tests/unit/source_code/notebooks/test_loader.py
Original file line number Diff line number Diff line change
@@ -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():

Expand All @@ -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
27 changes: 27 additions & 0 deletions tests/unit/source_code/notebooks/test_sources.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import codecs
import locale
from pathlib import Path
from unittest.mock import create_autospec

import pytest

Expand Down Expand Up @@ -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}"

0 comments on commit 6420e52

Please sign in to comment.