Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a new command databricks labs ucx lint-local-code #1594

Closed
wants to merge 5 commits into from

Conversation

jimidle
Copy link
Contributor

@jimidle jimidle commented Apr 30, 2024

A new command is created within the ucx cli that allows linting of local python files and local notebooks.

databricks labs ucx lint-local-code

Will prompt to ask if all files in the current directory and child directories should be linted. Upon confirmation, the
local python files and notebooks will be discovered and advisories will be raised for any issues identified.

Changes

Linked issues

Resolves #1541

Functionality

  • added relevant user documentation
  • added new CLI command
  • modified existing command: databricks labs ucx ...
  • added a new workflow
  • modified existing workflow: ...
  • added a new table
  • modified existing table: ...

Tests

  • manually tested
  • added unit tests
  • added integration tests
  • verified on staging environment (screenshot attached)

Copy link

codecov bot commented Apr 30, 2024

Codecov Report

Attention: Patch coverage is 43.75000% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 89.38%. Comparing base (ad1e2d9) to head (5a77bc7).

❗ Current head 5a77bc7 differs from pull request most recent head 386ee97. Consider uploading reports for the commit 386ee97 to get more accurate results

Files Patch % Lines
src/databricks/labs/ucx/source_code/files.py 33.33% 24 Missing and 2 partials ⚠️
src/databricks/labs/ucx/contexts/cli_command.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main    #1594       +/-   ##
=========================================
+ Coverage      0   89.38%   +89.38%     
=========================================
  Files         0       79       +79     
  Lines         0    10296    +10296     
  Branches      0     1820     +1820     
=========================================
+ Hits          0     9203     +9203     
- Misses        0      722      +722     
- Partials      0      371      +371     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/databricks/labs/ucx/cli.py Outdated Show resolved Hide resolved
src/databricks/labs/ucx/source_code/files.py Outdated Show resolved Hide resolved
src/databricks/labs/ucx/source_code/files.py Show resolved Hide resolved
jimidle added 5 commits May 2, 2024 14:31
…s labls ucx lint-local-code

  - Note that adding a lint method to local_file_migrator may or may not be
    the way to go.

Signed-off-by: Jim.Idle <[email protected]>
 - LocalFileLinter now receives the workspace client
 - Detects notebooks and calls the notebook linter for them
 - Otherwise receives the linters from the Languages instance
 - Languages is recreated for each lint so that the state is reset
   for each notebook.
 - Notebooks are assumed to have .py or .sql extensions with the
   file magic header

Signed-off-by: Jim.Idle <[email protected]>
 - TODO: There is only the workspace version of dependency checker right now
   so I use it to provie the mechanism, but when local NOtebook loading
   hits main, this will change to use that of course.

Signed-off-by: Jim.Idle <[email protected]>
@nfx
Copy link
Collaborator

nfx commented May 8, 2024

@ericvergnaud do it based on

class FileLinter:
_EXT = {
'.py': Language.PYTHON,
'.sql': Language.SQL,
}
def __init__(self, langs: Languages, path: Path):
self._languages: Languages = langs
self._path: Path = path
@cached_property
def _content(self) -> str:
return self._path.read_text()
def _file_language(self):
return self._EXT.get(self._path.suffix)
def _is_notebook(self):
language = self._file_language()
if not language:
return False
cell_language = CellLanguage.of_language(language)
return self._content.startswith(cell_language.file_magic_header)
def lint(self) -> Iterable[Advice]:
if self._is_notebook():
yield from self._lint_notebook()
else:
yield from self._lint_file()
def _lint_file(self):
language = self._file_language()
if not language:
yield Failure("unsupported-language", f"Cannot detect language for {self._path}", 0, 0, 1, 1)
try:
linter = self._languages.linter(language)
yield from linter.lint(self._content)
except ValueError as err:
yield Failure("unsupported-language", str(err), 0, 0, 1, 1)
def _lint_notebook(self):
notebook = Notebook.parse(self._path, self._content, self._file_language())
notebook_linter = NotebookLinter(self._languages, notebook)
yield from notebook_linter.lint()

@ericvergnaud
Copy link
Contributor

superseded by #1710 because the source branch is jim's and needs massive rebase

@nfx
Copy link
Collaborator

nfx commented May 21, 2024

closing as per last comment

@nfx nfx closed this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]: Create databricks labs ucx lint-local-code command
3 participants