Derive the dbt contract id from the manifest's project_name - #1403
Merged
Conversation
import_dbt_manifest called create_odcs() with no arguments, so odcs_helper's `id or "my-data-contract"` default applied to every dbt import regardless of the project. The project name was already read for odcs.name, so use it for the id too, normalising it the way the Power BI importer does. A manifest without a project_name still falls back to the previous default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1221.
import_dbt_manifestcalledcreate_odcs()with no arguments, soodcs_helper.create_odcs'sid=id or "my-data-contract"default applied to every dbt import. The project name was already being read one line later forodcs.name, so the information was there — just not used for the id.I normalise the id with
.lower().replace(" ", "-"), matchingpowerbi_importer.py, which is the only other importer that sets an id today. dbt project names are already snake_case by dbt's own naming rules, so this is a no-op for real manifests and only matters for hand-edited ones — happy to drop it and assignproject_nameverbatim if you'd rather keep it literal.A manifest with no
project_namestill falls back tomy-data-contract, so nothing changes for malformed input.Tests. The four expected fixtures all asserted
id: my-data-contract, so they are updated tojaffle_shop. Three new tests cover the behaviour directly: the id comes from the project name (checked against bothjaffle_shopandtest_project), it is slugified for a name with spaces and capitals, and it still falls back whenproject_nameis absent.Reverting only
dbt_importer.pywhile keeping the tests takestests/test_import_dbt.pyfrom 15 passed to 6 failed, so the new tests do guard the change rather than just restate it.No regressions.
pytest -k import --continue-on-collection-errorsgives 24 failed / 34 errors both before and after — all of them optional backends (S3, Trino, SQL Server) that need extras I don't have installed locally — while passes go 73 → 76, which is exactly the three new tests.uv run pytest)uv run ruff check --fix && uv run ruff format)docs/docs/imports/dbt.mdshows no sample contract, so nothing to change