[dagster-dbt] updates default_code_version_fn to take from dbt checksum if available
#26071
+17
−6
Conversation
This file contains 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
OwenKephart
requested changes
Nov 22, 2024
There was a problem hiding this comment.
Hi @marijncv ! Thanks for the submission. This definitely makes sense as a feature. If we were starting from scratch, it'd probably make sense to just use the checksum for all cases, but this would be a fairly rough breaking change.
Overall, I'm in favor of this change, just had a minor comment on style
| return hashlib.sha1(code.encode("utf-8")).hexdigest() | ||
|
|
||
| return ( | ||
| dbt_resource_props["checksum"]["checksum"] |
There was a problem hiding this comment.
just for style consistency, this can be return dbt_resource_props.get("checksum", {}).get("checksum")
There was a problem hiding this comment.
Makes sense, thanks!
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.
Summary & Motivation
This updates the
default_code_version_fnindagster-dbtto read fromdbt_resource_props["checksum"]["checksum"]if available (suggested here). This gives dbt seeds a valid model version.An alternative implementation could be to always read from the checksum field and ignore the
raw_sqlandraw_codefields. This would simplify the code version getter, but this would cause all current code versions of dbt models to be updated. Perhaps something for a breaking release?Closes #25947
How I Tested These Changes
Added test
Changelog
dagster-dbtnow gives dbt seeds a valid code version