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

[CT-2491] [Bug] Default --target-path should be relative to --project-dir, rather than current working directory #7465

Closed
2 tasks done
OwenKephart opened this issue Apr 26, 2023 · 13 comments · Fixed by #7706
Assignees
Labels
bug Something isn't working regression
Milestone

Comments

@OwenKephart
Copy link

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

Executing a command that targets a project in a specified --project-dir results in the json artifacts being written to current-working-directory/target/ rather than project-dir/target/.

Expected Behavior

In previous versions, specifying a target-path in your dbt_project.yml was relative to the project directory, rather than relative to the current working directory from which the command was executed.

Steps To Reproduce

In a new virtual environment (just using duckdb so I don't have to get docker involved):

pip install dbt-core==1.5.0rc2 dbt-duckdb==1.5.0rc1

gh repo clone dbt-labs/jaffle_shop

cd jaffle_shop

echo "jaffle_shop:
  outputs:
   dev:
     type: duckdb
     path: /tmp/dbt.duckdb
  target: dev" > profiles.yml
  
 cd some/other/path
 
 dbt build --profiles-dir ~/jaffle_shop --project-dir ~/jaffle_shop
 
 ls some/other/path 

At this point, you will observe that a new target directory has been created in some/other/path, and no target directory exists in ~/jaffle_shop.

Relevant log output

No response

Environment

- OS: Ventura 13.12
- Python: 3.9.10
- dbt: 1.5.0rc2

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

For this example, I'm using the duckdb adapter just for ease of reproducibility, but I don't think that has an impact on this specific behavior.

@OwenKephart OwenKephart added bug Something isn't working triage labels Apr 26, 2023
@github-actions github-actions bot changed the title [Bug] When no --target-path is specified, artifacts are written to the current working directory, rather than the default directory specified in dbt_project.yml [CT-2491] [Bug] When no --target-path is specified, artifacts are written to the current working directory, rather than the default directory specified in dbt_project.yml Apr 26, 2023
@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 26, 2023

@OwenKephart Thanks for opening, and for trying out the v1.5 RC!

We did need to make some low-level changes to how --target-path works in v1.5.0. This is called out in the migration guide:

Setting log-path and target-path in dbt_project.yml has been deprecated for consistency with other invocation-specific runtime configs (dbt-core#6882). We recommend setting via env var or CLI flag instead.

Now, the intent was to continue supporting this for the time being (with a deprecation warning: #7185), but it's possible that such functionality isn't supported when you're also running dbt in a separate working directory from dbt_project.yml (hence --project-dir).

I also understand that passing the --target-path flag is a bit annoying. Would it be possible for you to set as an env var (DBT_TARGET_PATH) instead?

@OwenKephart
Copy link
Author

@jtcohen6 Thanks for the quick response! (and oops on missing that bit in the migration guide).

The reasoning for removing target-path laid out in that PR definitely makes sense (and it's great that this can now be customized per-invocation! super useful).

Setting the DBT_TARGET_PATH env var has the nice property of being valid to set regardless of the user's dbt-core version (working on the dagster-dbt integration), so I'll swap over to that. Previously, I'd just been setting the subprocess's working directory to the project dir 😬

Of course, that will all become unnecessary once we port over to the dbtRunner stuff, which I'm very excited to do :)

I do still feel like the default behavior here should be to write target files to dbt_project_dir/target/ rather than current_working_directory/target/, but we're unblocked on this for now.

@jtcohen6
Copy link
Contributor

Of course, that will all become unnecessary once we port over to the dbtRunner stuff, which I'm very excited to do :)

Right on - if you didn't catch this in the docs, it would also be possible to set target_path by passing it as a kwarg to the invoke method: https://docs.getdbt.com/reference/programmatic-invocations#overriding-parameters

I do still feel like the default behavior here should be to write target files to dbt_project_dir/target/ rather than current_working_directory/target/, but we're unblocked on this for now.

This makes sense to me, and it looks like I said the same over in #6882 (comment). I'll keep this issue open accordingly.

@jtcohen6 jtcohen6 changed the title [CT-2491] [Bug] When no --target-path is specified, artifacts are written to the current working directory, rather than the default directory specified in dbt_project.yml [CT-2491] [Bug] Default --target-path should be relative to --project-dir, rather than current working directory Apr 27, 2023
@jtcohen6 jtcohen6 added this to the v1.5.x milestone Apr 27, 2023
@iknox-fa
Copy link
Contributor

Additional context from an internal Slack convo:
https://dbt-labs.slack.com/archives/C0131TY7EEA/p1682626884003739

TL;DR--
When target_path is not explicitly set via an env_var AND dbt is invoked from a working directory that isn't the project's root directory some artifacts are written to project_root/target and some are written to cwd/target

@ghost ghost mentioned this issue Apr 28, 2023
3 tasks
@iknox-fa
Copy link
Contributor

iknox-fa commented May 1, 2023

Per BLG-- Using contexts when changing dirs would make things less error prone

@jtcohen6
Copy link
Contributor

jtcohen6 commented May 3, 2023

Upgrading this to a regression given that it's causing surprising/difficult behavior for several users/plugins

@dwreeves
Copy link
Contributor

dwreeves commented May 8, 2023

Additional context from an internal Slack convo: https://dbt-labs.slack.com/archives/C0131TY7EEA/p1682626884003739

TL;DR-- When target_path is not explicitly set via an env_var AND dbt is invoked from a working directory that isn't the project's root directory some artifacts are written to project_root/target and some are written to cwd/target

I am running into this same issue but with the --target-path in the CLI. We are passing in ./dags/dbt/target as our target path. It appears to be the partial_parse.msgpack cache thingy is written to project_root/target and everything else is set to cwd/target. Note this file is written after everything runs, so the issue is a change of directory occurs at some point after the compilation.

Note that I was able to solve the issue by replacing --target-path "./dags/dbt/target" with --target-path "$(realpath ./dags/dbt/target)".

I am running dbt-core==1.5.0.

@dwreeves
Copy link
Contributor

dwreeves commented May 8, 2023

At least as far as the CLI is concerned, would it be possible to just fix the issue using a callback in the click.Option that wraps the user input with os.path.abspath()? If this is an acceptable solution, I can open a PR that does this.

(This however would be a bit of a bandaid and would not fix the underlying problem as far as other interfaces are concerned.)

@jaklan
Copy link

jaklan commented May 13, 2023

We have just encountered the same issue after the upgrade to 1.5 which broke our CI pipelines.

  • when having legacy target-path: "target" in common/dbt/dbt_project.yml, the below command:

    dbt compile --project-dir=common/dbt --profiles-dir=common/dbt

    results in:

    • partial_parse.msgpack written to common/dbt/target
    • compiled, graph.gpickle, and manifest.json written to target
  • when I remove the target-path config - the result is the same

  • when I both remove target-path and add --target-path=target- the result is the same

  • the only working solution is --target-path=<ABSOLUTE_PATH>/target

I also fully agree relative target-path should be relative to project-dir instead of cwd.

@jaklan
Copy link

jaklan commented May 19, 2023

We have just discovered the same issue happens when using --defer flag: --defer artifacts doesn't look at <project-dir>/artifacts anymore, but in <cwd>/artifacts. That's ofc unexpected and the previous behaviour should be preserved.

@jtcohen6
Copy link
Contributor

The fix for this will be in v1.5.2 (next patch)

@chamini2
Copy link
Contributor

Sounds great! Thank you

@britt-allen
Copy link

When will v1.5.2 be released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression
Projects
None yet
8 participants