From 1ba5094492a647b6b0290bbbed60c1b8bf4f6782 Mon Sep 17 00:00:00 2001 From: Victor Nilsson Date: Fri, 10 Feb 2023 15:14:59 +0100 Subject: [PATCH] fix!: add newline at the end of formatted notebooks (#36) Fixes #37 and closes #38. BREAKING CHANGE: All notebooks will now have a newline character `\n` at the end of the file. --- blackbricks/blackbricks.py | 2 +- blackbricks/cli.py | 1 - blackbricks/files.py | 26 +++++++++++++++++++++++--- test_notebooks/test.py | 2 +- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/blackbricks/blackbricks.py b/blackbricks/blackbricks.py index 89b97a3..8ae6f8c 100644 --- a/blackbricks/blackbricks.py +++ b/blackbricks/blackbricks.py @@ -72,7 +72,7 @@ def format_str(content: str, config: FormatConfig = FormatConfig()) -> str: output_ws_normalized += line + "\n" - return output_ws_normalized.strip() + return output_ws_normalized.strip() + "\n" def _format_sql_cell( diff --git a/blackbricks/cli.py b/blackbricks/cli.py index 7e17cd7..778cd83 100644 --- a/blackbricks/cli.py +++ b/blackbricks/cli.py @@ -31,7 +31,6 @@ def process_files( n_notebooks = 0 for file_ in files: - try: content = file_.content except UnicodeDecodeError: diff --git a/blackbricks/files.py b/blackbricks/files.py index 694aaef..48490d9 100644 --- a/blackbricks/files.py +++ b/blackbricks/files.py @@ -42,10 +42,32 @@ class RemoteNotebook(File): @property def content(self) -> str: - return self.api_client.read_notebook(self.path) + """ + Get the content of the notebook as a string. + + Note: Databricks will not include a trailing newlines from a notebook. + That is, even if you checkout a notebook from a repo where there is a trailing + newline, Databricks won't "show" an empty line in the UI. And similarly, this + API doesn't include it. This contrasts with the prefered style of terminating + the files with a newline, as enforced by black and blackbricks. To avoid + perpetually reporting diffs due to newlines, we add a trailing newline to the + content, _assuming_ that the + we assume it to be present and add it back if it is missing. Failing to do this + will cause blackbricks to perpetually report that it wants to add a trailing + newline to remote notebooks. + """ + return self.api_client.read_notebook(self.path) + "\n" @content.setter def content(self, new_content: str, /) -> None: + """ + Set the content of the notebook to the given string. + + Note: We do _not_ need to do the inverse handling of trailing newlines as in + the getter. The new content here is presumably the output of blackbricks, and + we should let Databricks import that from the same source as it would find by + checking out a repo notebook that has been formatted with blackbricks. + """ self.api_client.write_notebook(self.path, new_content) @@ -71,12 +93,10 @@ def resolve_filepaths(paths: List[str]) -> List[str]: raise typer.Exit(1) if os.path.isdir(path): - # Recursively add all the files/dirs in path to the paths to be consumed. paths.extend([os.path.join(path, f) for f in os.listdir(path)]) else: - file_paths.append(path) return file_paths diff --git a/test_notebooks/test.py b/test_notebooks/test.py index e8380ed..d1877dd 100644 --- a/test_notebooks/test.py +++ b/test_notebooks/test.py @@ -78,4 +78,4 @@ def test_func(input_param): # MAGIC %sql # MAGIC SELECT id # MAGIC FROM test_table -# MAGIC LIMIT 1 \ No newline at end of file +# MAGIC LIMIT 1