Fix for dbt compile overwriting source files

[Myles1]

Resolves #10886

Problem

Running dbt compile can overwrite source files with the newly compiled sql

Solution

Update path building to avoid issues with absolute paths.

The issue appears on this line:

dbt-core/core/dbt/contracts/graph/nodes.py

Line 267 in a0674db

target_write_path = os.path.join(target_path, subdirectory, self.package_name, path)

Consider the case where path is an absolute path.
For example, running this will give the absolute path as the return value, instead of the expected relative path that will be used as the output target file.

>>> os.path.join("target_path", "subdirectory", "package_name", "/absolute/path/project/models/my_model.sql")
'/absolute/path/project/models/my_model.sql'

Following the change in this PR, we can expect this as the result:

>>> os.path.join("target_path", "subdirectory", "package_name", "models/my_model.sql")
'target_path/subdirectory/package_name/models/my_model.sql'

Checklist

• I have read the contributing guide and understand what's expected of me.
• I have run this code in development, and it appears to resolve the stated issue.
• This PR includes tests, or tests are not required or relevant for this PR.
• This PR has no interface changes (e.g., macros, CLI, logs, JSON artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX.
• This PR includes type annotations for new and modified functions.

[cla-bot]

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @Myles1

[Myles1]

CLA should be signed now

[dbeatty10]

Thanks for opening this PR @Myles1 !

Could you add a functional test for this case?

One place it could go initially is tests/functional/adapter/simple_seed/test_seed.py (when this is reviewed by one of our software engineers, they might request to remove it to a different location).

I'm guessing the test could look something like this (which I didn't try out at all, so probably doesn't work as-is):

import Path
from tests.functional.adapter.simple_seed.seeds import (
    seed__with_dots_csv,
)

class BaseTestAbsoluteSeedPaths:
    @pytest.fixture(scope="class")
    def project_config_update(self, project_root):
    
        # Assign an absolute path to seed-paths
        return {
            "seed-paths": [Path(project_root, "seeds")],
        }

    @pytest.fixture(scope="class")
    def seeds(self):
        return {"my_seed.csv": seed__with_dots_csv}

    def test_absolute_seeds_paths(self, project):
        results = run_dbt(["seed"])
        assert len(results) == 1
        # Should not fail due to file being overwritten
        results = run_dbt(["seed"])

[dbeatty10]

Could you restore the original index.html (assuming this was an accidental change)?

[Myles1]

Whoops. Sure: dc370bf

[dbeatty10]

I wasn't able to get an error with dbt compile, but I was with dbt seed and dbt build.

Suggested change

	body: Prevent dbt compile from overwriting files that are passed in as absolute paths
	body: Prevent `dbt seed` from overwriting files when `seed-path` contains an absolute path

[Myles1]

Happy to update it if needed. Lmk after giving this a shot: #10886 (comment)

[dbeatty10]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.31%. Comparing base (ba6c7ba) to head (2b18d70).
Report is 4 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (ba6c7ba) and HEAD (2b18d70). Click for more details.

HEAD has 4 uploads less than BASE

Flag	BASE (ba6c7ba)	HEAD (2b18d70)
integration	5	1

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10887      +/-   ##
==========================================
- Coverage   89.18%   79.31%   -9.88%     
==========================================
  Files         183      183              
  Lines       23443    23443              
==========================================
- Hits        20908    18593    -2315     
- Misses       2535     4850    +2315     

Flag	Coverage Δ
integration	72.38% <100.00%> (-14.09%)	⬇️
unit	62.12% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
Unit Tests	62.12% <0.00%> (ø)
Integration Tests	72.38% <100.00%> (-14.09%)	⬇️

[Myles1]

Could you add a functional test for this case?

Sure thing. Here's a (very) slightly modified version of your test that seems to do the trick: 9ae8aae
I can confirm that this test fails when the change in this PR is reverted.

[martinazapletalova]

Hi, is anybody working on fixing this anymore? We run into this issue even with dbt compile which rewrites all of our defined sqls and this fix is fixing it. Thank you.

[Myles1]

Hi, is anybody working on fixing this anymore? We run into this issue even with dbt compile which rewrites all of our defined sqls and this fix is fixing it. Thank you.

Not sure. I'm just waiting on a reply from @dbeatty10 or another maintainer.