-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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-599] [Bug] Upgrading from 1.0 to 1.1 breaks defer/state #5213
Comments
@pecigonzalo Thanks for opening. I totally appreciate where you're coming from here. This is on us (me especially) for failing to clearly set this expectation for metadata interfaces during minor version upgrades—in particular, calling out that it does have (one-time) implications for users of advanced features that depend on those interfaces, such as artifact-powered node selection. As soon as you're able to compare to a v5 manifest, produced by v1.1,
In every minor version that has included a bump to the Here's what we have documented today: https://docs.getdbt.com/docs/guides/understanding-state#notes:
In the short term, I think the right move here is to:
In the longer term, we're looking to create more distance between our internal objects and the external artifacts that dbt produces, so that behind-the-scenes changes do not require us to bump the manifest version in each new minor version (#4617). I'd really like to get away from the 1:1 relationship we've had to date, of new dbt minor version == new manifest version. |
Hey @jtcohen6 thanks for the through answer.
This was not the case in our experience, 0.20 was able to read 0.19 for For 1.0 -> 1.1 this was not the case.
I expected that any version of DBT that bumps the schema, would be compatible with schema version N-1, so we can migrate without breakage. Right now, this incompatibility means our PR to bump the version fails, as This was also our experience as mentioned when doing |
Thanks for the quick response @pecigonzalo! I just want to confirm that this was the same experience when upgrading older versions. Here's what I did to reproduce:
Given how small the change was between |
That is odd, it works for me no problem across versions. eg.
|
I see the mistake I made tho, since we did this Nevertheless, that is a major release and while ideally it should be able to read, its clearer that is a breaking change. I was expecting this minor to behave the same way as I think it would be great if DBT would be able to convert between minor versions, read old -> create new, this facilities CI processes. Right now I would have to create a special case for this so we can still test or test manually and then merge so CI continues to work. Thanks for your help @jtcohen6 |
Right now we use the current Python code to load the artifacts. If we didn't check the schema versions, we'd end up having cryptic errors about unable to deserialize. We can't have multiple Python versions of the classes. It's possible that some schema changes would not cause serialization errors, if the new attributes had defaults or were optional. We'd have to do testing against the previous version to see if it would load cleanly and then add an exception to the 'read_and_check_versions' function. We could also add code to modify the loaded json artifacts and convert them to something compatible. The complexity of doing that would depend on the types of the changes. |
Desirable end state: Users with jobs leveraging It feels uncontroversial to me that we should support that behavior for manifest So, as a way of generalizing that, here's what I'm thinking:
I've been poking around the code a bit, and this feels feasible. I'll open a draft PR for comment. At the same time as making this change, we should also aim to avoid changes to |
@jtcohen6 thanks for the writeup/summary.
This part is a bit unclear. Could you clarify it? |
@pecigonzalo Thanks for reading, and catching me there! I was missing a very important instance of the word |
Is there an existing issue for this?
Current Behavior
After the merge of #5032 manifest version was bumped to v5, but this change was not made backwards compatible like in other version (0.19 -> 0.20 -> 0.21 -> 1.0), and its a breaking change for a
minor
release.This causes
Expected Behavior
DBT version 1.1 is able to run with
--defer
or old state reference to v4.Steps To Reproduce
dbt ls
target/
toold/
dbt run --defer --state old/
Relevant log output
The text was updated successfully, but these errors were encountered: