Skip to content

Conversation

@ericvergnaud
Copy link
Contributor

@ericvergnaud ericvergnaud commented Sep 2, 2024

Current implementation does not support exclusions when running 'fmt' command.
This is blocking when verifying notebooks in ucx because the fmt sqlglot converts single line comments -- comment into inlined comments /* comment */ thus misleading the verifier.
This PR implements exclusions when running 'fmt' command.
Example cmd line is 'databricks labs lsql fmt --exclude tests/units/samples'
The added unit test is verified manually.
Fixes #261

Signed-off-by: Eric Vergnaud <eric.vergnaud@databricks.com>
@github-actions
Copy link

github-actions bot commented Sep 2, 2024

✅ 34/34 passed, 3 skipped, 22m49s total

Running from acceptance #367

@nfx nfx disabled auto-merge September 3, 2024 14:24
@nfx nfx merged commit 0239089 into main Sep 3, 2024
@nfx nfx deleted the add-support-for-exclusiions-in-fmt-command branch September 3, 2024 14:24
@nfx nfx mentioned this pull request Sep 3, 2024
nfx added a commit that referenced this pull request Sep 3, 2024
* Make hatch a prerequisite ([#259](#259)). In this commit, Eric Vergnaud has introduced a change to make the installation of `hatch` version 1.9.4 a prerequisite for the project to avoid errors related to `pip` command recognition. The Makefile has been updated to handle the installation of hatch automatically, and the `hatch env create` command is now used instead of `pip install hatch==1.7.0`. This change ensures that the development environment is consistent and reliable by maintaining the correct version of hatch and automatically handling its installation. Additionally, the `.venv/bin/python` and `dev` targets have been updated accordingly to reflect these changes. This commit also formats all files using the `make dev fmt` command, which helps maintain consistent code formatting throughout the project.
* add support for exclusions in `fmt` command ([#263](#263)). In this release, we have added support for exclusions to the `fmt` command in the 'databricks/labs/lsql/cli.py' module. This feature allows users to specify a list of directories or files to exclude while formatting SQL files, which is particularly useful when verifying SQL notebooks in ucx. The `fmt` command now accepts a new optional parameter 'exclude', which accepts an iterable of strings that specify the relative paths to exclude. Any `sql_file` that is a descendant of any `exclusion` is skipped during formatting. The exclusions are implemented by converting the relative paths into `Path` objects. This change addresses the issue where single line comments are converted into inlined comments, causing misinterpretation. The added unit test is manually verified, and this pull request fixes issue [#261](#261). This feature was authored and co-authored by Eric Vergnaud.
nfx added a commit that referenced this pull request Sep 3, 2024
* Make hatch a prerequisite
([#259](#259)). In this
commit, Eric Vergnaud has introduced a change to make the installation
of `hatch` version 1.9.4 a prerequisite for the project to avoid errors
related to `pip` command recognition. The Makefile has been updated to
handle the installation of hatch automatically, and the `hatch env
create` command is now used instead of `pip install hatch==1.7.0`. This
change ensures that the development environment is consistent and
reliable by maintaining the correct version of hatch and automatically
handling its installation. Additionally, the `.venv/bin/python` and
`dev` targets have been updated accordingly to reflect these changes.
This commit also formats all files using the `make dev fmt` command,
which helps maintain consistent code formatting throughout the project.
* add support for exclusions in `fmt` command
([#263](#263)). In this
release, we have added support for exclusions to the `fmt` command in
the 'databricks/labs/lsql/cli.py' module. This feature allows users to
specify a list of directories or files to exclude while formatting SQL
files, which is particularly useful when verifying SQL notebooks in ucx.
The `fmt` command now accepts a new optional parameter 'exclude', which
accepts an iterable of strings that specify the relative paths to
exclude. Any `sql_file` that is a descendant of any `exclusion` is
skipped during formatting. The exclusions are implemented by converting
the relative paths into `Path` objects. This change addresses the issue
where single line comments are converted into inlined comments, causing
misinterpretation. The added unit test is manually verified, and this
pull request fixes issue
[#261](#261). This feature
was authored and co-authored by Eric Vergnaud.
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] Add support for --exclude option to fmt CLI command

3 participants