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

Dcv 1682 possibility to change the dbt root for all hooks #138

Conversation

BAntonellini
Copy link
Collaborator

@BAntonellini BAntonellini commented Jun 23, 2023

This PR introduces the possibility of specifying your dbt project for all hooks.

This can be done by setting dbt-project-dir in the user's .dbt-checkpoint.yaml: dbt-project-dir: my_project

With that, we modified the get manifest and get catalog behavior to use this content.

Now there is a precedence in flags and config usage:

  1. custom --manifest path/to/manifest.json or --cmd-flags ++project+dir
  2. if used, config's file my_project/target/manifest.json is passed to manifest or --project-dir in dbt commands
  3. default flags. For example, in --manifest: target/manifest.json

This way, if both approaches are used, args: [] content takes precedence over config file one.

With these changes, it's no longer needed to provide a --manifest or --catalog to each of the check[...] hooks and repeating the same args multiple times, leaving the following as a working example of .pre-commit-config.yaml:

repos:
  - repo: https://github.com/BAntonellini/dbt-checkpoint
    rev: 021c9cd38c518a4fc5d4748a317d573514b89186

    hooks:
      - id: check-script-semicolon
      - id: check-script-has-no-table-name
      - id: dbt-test
      - id: dbt-docs-generate
        args: ["--cmd-flags", "++no+compile"]
      - id: check-model-has-all-columns
      - id: check-model-columns-have-desc
      - id: check-source-table-has-description
      - id: check-script-ref-and-source
      - id: check-model-has-description

@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (f58bd0d) 98.58% compared to head (53b9031) 98.61%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #138      +/-   ##
==========================================
+ Coverage   98.58%   98.61%   +0.02%     
==========================================
  Files          47       47              
  Lines        2269     2311      +42     
  Branches      304      309       +5     
==========================================
+ Hits         2237     2279      +42     
  Misses         18       18              
  Partials       14       14              
Impacted Files Coverage Δ
dbt_checkpoint/check_column_desc_are_same.py 100.00% <100.00%> (ø)
dbt_checkpoint/check_column_name_contract.py 100.00% <100.00%> (ø)
dbt_checkpoint/check_macro_arguments_have_desc.py 96.22% <100.00%> (ø)
dbt_checkpoint/check_macro_has_description.py 100.00% <100.00%> (ø)
dbt_checkpoint/check_model_columns_have_desc.py 96.22% <100.00%> (ø)
dbt_checkpoint/check_model_has_all_columns.py 96.61% <100.00%> (ø)
dbt_checkpoint/check_model_has_description.py 100.00% <100.00%> (ø)
dbt_checkpoint/check_model_has_meta_keys.py 100.00% <100.00%> (ø)
dbt_checkpoint/check_model_has_properties_file.py 100.00% <100.00%> (ø)
dbt_checkpoint/check_model_has_tests.py 100.00% <100.00%> (ø)
... and 35 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@BAntonellini BAntonellini marked this pull request as ready for review June 26, 2023 17:51
@BAntonellini
Copy link
Collaborator Author

@dbt-checkpoint/montreal-analytics @dbt-checkpoint/datacoves

tests created, please provide any feedback as it seems this will be a nice addition as discussed in Issue #39

…DCV-1682-possibility-to-change-the-dbt-root-for-all-hooks
@ssassi ssassi requested a review from JFrackson June 29, 2023 18:24
@ssassi
Copy link
Collaborator

ssassi commented Jun 29, 2023

LGTM

Copy link
Collaborator

@JFrackson JFrackson left a comment

Choose a reason for hiding this comment

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

lgtm

@BAntonellini BAntonellini merged commit 07726d8 into dbt-checkpoint:main Jul 6, 2023
7 checks passed
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.

None yet

4 participants