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

Further improvements to dbt sort order linter #647

Conversation

jeancochrane
Copy link
Contributor

@jeancochrane jeancochrane commented Nov 18, 2024

#644 got merged in before we could implement two pieces of feedback:

  • Adjusting the exclude attribute of the pre-commit hook to be more precise and only exclude files in a directory matching the string venv/ rather than any file containing the substring venv, which could exist without being in a venv dir (comment)
  • Adding a docstring to explain the different cases in which the linter runs, and how it expects arguments (comments)

In the course of implementing these two pieces of feedback, I also noticed that the logic to check for properly sorted tests is only looking at data tests and so is ignoring the unit tests we added in #638. This PR also tweaks that logic so that we are properly linting unit tests.

Sample output including both types of unsorted tests:

$ python dbt/scripts/check_sort_dbt_yaml_files.py dbt/models/default/schema/default.vw_pin_universe.yml
In file: dbt/models/default/schema/default.vw_pin_universe.yml
Top level: models
data_tests in this group are not sorted:
---> default_vw_pin_universe_unique_by_14_digit_pin_and_year
---> default_vw_pin_universe_row_count
----------------------------------------
In file: dbt/models/default/schema/default.vw_pin_universe.yml
unit_tests in this group are not sorted:
---> default_vw_pin_universe_filters_invalid_pins
---> default_vw_pin_universe_class_strips_non_alphanumerics
----------------------------------------

The following files have unsorted tests:
dbt/models/default/schema/default.vw_pin_universe.yml (2)

@jeancochrane jeancochrane marked this pull request as ready for review November 18, 2024 21:02
@jeancochrane jeancochrane requested a review from a team as a code owner November 18, 2024 21:02
@jeancochrane jeancochrane requested a review from dfsnow November 18, 2024 21:02
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the cleanup @jeancochrane 🙏 Sorry for the early merge!

@jeancochrane jeancochrane merged commit d87dd94 into master Nov 18, 2024
7 checks passed
@jeancochrane jeancochrane deleted the jeancochrane/further-improvements-to-check-sort-dbt-yaml-files branch November 18, 2024 21:18
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.

2 participants