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

fix: dbt/target path in clean-targets in dbt_project.yml #56

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nivinsrinivas
Copy link

@nivinsrinivas nivinsrinivas commented Feb 2, 2024

Issue

meltano/meltano#8391

Summary

  • Running meltano invoke dbt-postgres:run while following doc throws an error due to incorrect path specified in dbt_project.yml

Changes made to fix the issue

  • Modified the dbt_project.yml file. Removed the ../.meltano/transformers/dbt/target path to unblock basic functionality of dbt-postgres like run, compile and test

@nivinsrinivas nivinsrinivas requested a review from a team as a code owner February 2, 2024 19:24
@nivinsrinivas nivinsrinivas changed the title Fix dbt/target path in clean-targets in dbt_project.yml bugfix: Fix dbt/target path in clean-targets in dbt_project.yml Feb 2, 2024
@nivinsrinivas nivinsrinivas changed the title bugfix: Fix dbt/target path in clean-targets in dbt_project.yml fix: dbt/target path in clean-targets in dbt_project.yml Feb 2, 2024
@nivinsrinivas
Copy link
Author

nivinsrinivas commented Feb 2, 2024

Thanks @nivinsrinivas!

Can you confirm that with this change, does dbt clean runs successfully and deletes the right files, or does it simply not delete anything?

For ref:

hey @edgarrmondragon, yes you are right, though this fixes the dbt error, it doesn't seem to delete the compiled sql files in .meltano/transformers/dbt/target

Need to check why dbt fails to cleanup files in this target but cleans up just fine in other clean-targets.

@nivinsrinivas
Copy link
Author

nivinsrinivas commented Feb 8, 2024

hey edgarrmondragon
I tried the --no-clean-project-files-only flag, but it still doesn't work
(mlt) nivinsrinivas@MBP meltano_project % meltano invoke dbt-postgres:clean --no-clean-project-files-only

Extension executing dbt clean... 23:17:16 Running with dbt=1.7.7 23:17:16 [WARNING]: Deprecated functionality The target-pathconfig indbt_project.yml has been deprecated, and will no longer be supported in a future version of dbt-core. If you wish to write dbt artifacts to a custom directory, please use the --target-path CLI flag or DBT_TARGET_PATH env var instead. 23:17:16 Encountered an error: Runtime Error dbt will not clean the following directories outside the project: ['/Users/nivinsrinivas/meltano_project/.meltano/transformers/dbt/target']

This clean command blocks all the dbt-postgres commands like compile, run and test.
I think its better to get rid of the ../.meltano/transformers/dbt/target path from clean-targets to unblock the basic functionality of dbt-postgres.

What do you think?

@edgarrmondragon
Copy link
Contributor

@nivinsrinivas I think there's a few changes we can make:

  1. (optionally) update the EDK's run and run_and_log to accept a raise: bool = True keyword-only argument: https://github.com/meltano/edk/blob/48e95e4e2d0b27b38da211a8f7c5333fc88b89f7/meltano/edk/process.py#L57

  2. make this

    self.dbt_invoker.run_and_log("clean")

    # pass raise=False if the above is done
    self.dbt_invoker.run_and_log("clean", "--no-clean-project-files-only")

I think we can ignore the target path warning for now

@nivinsrinivas
Copy link
Author

nivinsrinivas commented Feb 10, 2024

@nivinsrinivas I think there's a few changes we can make:

  1. (optionally) update the EDK's run and run_and_log to accept a raise: bool = True keyword-only argument: https://github.com/meltano/edk/blob/48e95e4e2d0b27b38da211a8f7c5333fc88b89f7/meltano/edk/process.py#L57

  2. make this

    self.dbt_invoker.run_and_log("clean")

    # pass raise=False if the above is done
    self.dbt_invoker.run_and_log("clean", "--no-clean-project-files-only")

I think we can ignore the target path warning for now

@edgarrmondragon
Raised a PR for EDK repo: meltano/edk#206
Once this is merged, I'll make changes here.

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