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-599] [Bug] Upgrading from 1.0 to 1.1 breaks defer/state #5213

Closed
1 task done
pecigonzalo opened this issue May 4, 2022 · 9 comments · Fixed by #5346
Closed
1 task done

[CT-599] [Bug] Upgrading from 1.0 to 1.1 breaks defer/state #5213

pecigonzalo opened this issue May 4, 2022 · 9 comments · Fixed by #5346
Labels
artifacts bug Something isn't working state Stateful selection (state:modified, defer) user docs [docs.getdbt.com] Needs better documentation

Comments

@pecigonzalo
Copy link

pecigonzalo commented May 4, 2022

Is there an existing issue for this?

  • I have searched the existing issues

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 a schema version of "https://schemas.getdbt.com/dbt/manifest/v5.json" in /root/dbt_repo/manifest.json, but found "https://schemas.getdbt.com/dbt/manifest/v4.json". Are you running with a different version of dbt? 

Expected Behavior

DBT version 1.1 is able to run with --defer or old state reference to v4.

Steps To Reproduce

  1. Install dbt 1.0
  2. In some sample repository/project run dbt ls
  3. Move target/ to old/
  4. Install dbt 1.1
  5. Run dbt run --defer --state old/

Relevant log output

Expected a schema version of "https://schemas.getdbt.com/dbt/manifest/v5.json" in /root/dbt_repo/manifest.json, but found "https://schemas.getdbt.com/dbt/manifest/v4.json". Are you running with a different version of dbt? 


### Environment

N/A

### What database are you using dbt with?

snowflake

### Additional Context

N/A
@pecigonzalo pecigonzalo added bug Something isn't working triage labels May 4, 2022
@github-actions github-actions bot changed the title [Bug] Upgrading from 1.0 to 1.1 breaks defer/state [CT-599] [Bug] Upgrading from 1.0 to 1.1 breaks defer/state May 4, 2022
@jtcohen6 jtcohen6 added artifacts state Stateful selection (state:modified, defer) and removed triage labels May 4, 2022
@jtcohen6
Copy link
Contributor

jtcohen6 commented May 4, 2022

@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, state:modified will start working again. It's only while in the process of upgrading that this issue occurs.

this change was not made backwards compatible like in other version (0.19 -> 0.20 -> 0.21 -> 1.0)

In every minor version that has included a bump to the manifest.json version, dbt has raised this same clear error message. It will always raise this error message when trying to compare against an incompatible manifest. The thing that is different this time is that the upgrade is from 1.0 and 1.1; I get the perception that this is a breaking change. As soon as you can compare to a past / production run on 1.1, it will start working again.

Here's what we have documented today:

https://docs.getdbt.com/docs/guides/understanding-state#notes:

The --state artifacts must be of schema versions that are compatible with the currently running dbt version.

https://docs.getdbt.com/docs/guides/migration-guide/upgrading-to-v1.1#for-consumers-of-dbt-artifacts-metadata

For consumers of dbt artifacts (metadata)
The manifest schema version will be updated to v5. The only change is to the default value of config for parsed nodes.

In the short term, I think the right move here is to:

  • Update the migration guide, to make clear that, while there are no breaking changes for project code, the manifest version change has one-time implications for folks using state-powered selection
  • Update "About dbt Core versions" to make clear that, while there are no breaking changes for project/package code in minor versions after v1.0, we are reserving the right to make and communicate changes to (a) the adapter plugin interface, and (b) metadata interfaces. The latter will always be reflected in an explicit version bump.

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.

@pecigonzalo
Copy link
Author

Hey @jtcohen6 thanks for the through answer.

this change was not made backwards compatible like in other version (0.19 -> 0.20 -> 0.21 -> 1.0)

In every minor version that has included a bump to the manifest.json version, dbt has raised this same clear error message. It will always raise this error message when trying to compare against an incompatible manifest. The thing that is different this time is that the upgrade is from 1.0 and 1.1; I get the perception that this is a breaking change. As soon as you can compare to a past / production run on 1.1, it will start working again.

This was not the case in our experience, 0.20 was able to read 0.19 for --defer, so in our way to 1.0, we did all the intermediate steps, and this worked.

For 1.0 -> 1.1 this was not the case.

The --state artifacts must be of schema versions that are compatible with the currently running dbt version.

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 --defer/--state do not like the old manifest.

This was also our experience as mentioned when doing 0.19 -> 0.20 and then 0.20 -> 0.21 and then 0.21 -> 1.0.

@jtcohen6
Copy link
Contributor

jtcohen6 commented May 4, 2022

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:

  1. Install dbt-postgres==0.21.1
  2. Initialize a simple project
  3. dbt run with v0.21.1
  4. Move the resulting target/manifest.json to a state/ folder
  5. Upgrade to dbt-postgres~=1.0.0
  6. dbt run -s state:modified --state state/
$ dbt run -s state:modified --state state/
12:34:53  [WARNING]: Deprecated functionality
The `source-paths` config has been renamed to `model-paths`. Please update your
`dbt_project.yml` configuration to reflect this change.
12:34:53  [WARNING]: Deprecated functionality
The `data-paths` config has been renamed to `seed-paths`. Please update your
`dbt_project.yml` configuration to reflect this change.
12:34:53  Encountered an error:
Runtime Error
  Expected a schema version of "https://schemas.getdbt.com/dbt/manifest/v4.json" in state/manifest.json, but found "https://schemas.getdbt.com/dbt/manifest/v3.json". Are you running with a different version of dbt?

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.

Given how small the change was between v4 and v5, we can look into the complexity of doing this. I agree it would make for a smoother upgrade experience, if new artifact versions could declare a set of older artifact versions for which they are backwards-compatible. (This won't always be the case.) I'm queueing this up for the Core - Language team to consider and estimate.

@pecigonzalo
Copy link
Author

That is odd, it works for me no problem across versions. eg. 0.20 -> 0.21

❯ dbt debug
Running with dbt=0.20.1
dbt version: 0.20.1
python version: 3.9.9
python path: /tmp/tmp.ruXcQQJkXw/.direnv/python-3.9.9/bin/python3
os info: Linux-5.13.0-1022-aws-x86_64-with-glibc2.33
Using profiles.yml file at /home/ubuntu/.dbt/profiles.yml
Using dbt_project.yml file at /tmp/tmp.ruXcQQJkXw/dbt_project.yml

Configuration:
  profiles.yml file [OK found and valid]
  dbt_project.yml file [OK found and valid]

Required dependencies:
 - git [OK found]

Connection:
  host: localhost
  port: 5432
  user: root
  database: dbt
  schema: public
  search_path: None
  keepalives_idle: 0
  sslmode: None
  Connection test: OK connection ok

❯ dbt compile
Running with dbt=0.20.1
Unable to do partial parsing because target/partial_parse.msgpack not found
Found 5 models, 20 tests, 0 snapshots, 0 analyses, 147 macros, 0 operations, 3 seed files, 0 sources, 0 exposures

12:59:55 | Concurrency: 1 threads (target='local')
12:59:55 |
12:59:55 | Done.

…/tmp.ruXcQQJkXw on  main [!?] via  v3.9.9 (python-3.9.9) took 3s
❯ dbt compile

❯ ls target
 compiled   graph.gpickle   manifest.json   partial_parse.msgpack   run_results.json

❯ mv target old

[...] Install 0.21.1

❯ dbt debug
Running with dbt=0.21.1
dbt version: 0.21.1
python version: 3.9.9
python path: /tmp/tmp.ruXcQQJkXw/.direnv/python-3.9.9/bin/python3
os info: Linux-5.13.0-1022-aws-x86_64-with-glibc2.33
Using profiles.yml file at /home/ubuntu/.dbt/profiles.yml
Using dbt_project.yml file at /tmp/tmp.ruXcQQJkXw/dbt_project.yml

Configuration:
  profiles.yml file [OK found and valid]
  dbt_project.yml file [OK found and valid]

Required dependencies:
 - git [OK found]

Connection:
  host: localhost
  port: 5432
  user: root
  database: dbt
  schema: public
  search_path: None
  keepalives_idle: 0
  sslmode: None
  Connection test: [OK connection ok]

All checks passed!

❯ dbt compile --state old
Running with dbt=0.21.1
Partial parsing enabled: 0 files deleted, 0 files added, 0 files changed.
Partial parsing enabled, no changes found, skipping parsing
Found 5 models, 20 tests, 0 snapshots, 0 analyses, 162 macros, 0 operations, 3 seed files, 0 sources, 0 exposures

13:02:11 | Concurrency: 1 threads (target='local')
13:02:11 |
13:02:11 | Done.

❯ dbt build --defer --state old
Running with dbt=0.21.1
Partial parsing enabled: 0 files deleted, 0 files added, 0 files changed.
Partial parsing enabled, no changes found, skipping parsing
Found 5 models, 20 tests, 0 snapshots, 0 analyses, 162 macros, 0 operations, 3 seed files, 0 sources, 0 exposures

13:03:17 | Concurrency: 1 threads (target='local')
13:03:17 |
13:03:17 | 1 of 28 START seed file public.raw_customers......................... [RUN]
* Deprecation Warning: The quote_columns parameter was not set for seeds, so the
default value of False was chosen. The default will change to True in a future
release.

For more information, see:
https://docs.getdbt.com/v0.15/docs/seeds#section-specify-column-quoting
13:03:17 | 1 of 28 OK loaded seed file public.raw_customers..................... [INSERT 100 in 0.15s]
13:03:17 | 2 of 28 START seed file public.raw_orders............................ [RUN]
13:03:17 | 2 of 28 OK loaded seed file public.raw_orders........................ [INSERT 99 in 0.10s]
13:03:17 | 3 of 28 START seed file public.raw_payments.......................... [RUN]
13:03:17 | 3 of 28 OK loaded seed file public.raw_payments...................... [INSERT 113 in 0.10s]
13:03:17 | 4 of 28 START view model public.stg_customers........................ [RUN]
13:03:17 | 4 of 28 OK created view model public.stg_customers................... [CREATE VIEW in 0.05s]
13:03:17 | 5 of 28 START view model public.stg_orders........................... [RUN]
13:03:17 | 5 of 28 OK created view model public.stg_orders...................... [CREATE VIEW in 0.02s]
13:03:17 | 6 of 28 START view model public.stg_payments......................... [RUN]
13:03:17 | 6 of 28 OK created view model public.stg_payments.................... [CREATE VIEW in 0.02s]
13:03:17 | 7 of 28 START test not_null_stg_customers_customer_id................ [RUN]
13:03:17 | 7 of 28 PASS not_null_stg_customers_customer_id...................... [PASS in 0.04s]
13:03:17 | 8 of 28 START test unique_stg_customers_customer_id.................. [RUN]
13:03:17 | 8 of 28 PASS unique_stg_customers_customer_id........................ [PASS in 0.02s]
13:03:17 | 9 of 28 START test accepted_values_stg_orders_status__placed__shipped__completed__return_pending__returned [RUN]
13:03:17 | 9 of 28 PASS accepted_values_stg_orders_status__placed__shipped__completed__return_pending__returned [PASS in 0.02s]
13:03:17 | 10 of 28 START test not_null_stg_orders_order_id..................... [RUN]
13:03:17 | 10 of 28 PASS not_null_stg_orders_order_id........................... [PASS in 0.01s]
13:03:17 | 11 of 28 START test unique_stg_orders_order_id....................... [RUN]
13:03:17 | 11 of 28 PASS unique_stg_orders_order_id............................. [PASS in 0.01s]
13:03:17 | 12 of 28 START test accepted_values_stg_payments_payment_method__credit_card__coupon__bank_transfer__gift_card [RUN]
13:03:17 | 12 of 28 PASS accepted_values_stg_payments_payment_method__credit_card__coupon__bank_transfer__gift_card [PASS in 0.02s]
13:03:17 | 13 of 28 START test not_null_stg_payments_payment_id................. [RUN]
13:03:17 | 13 of 28 PASS not_null_stg_payments_payment_id....................... [PASS in 0.02s]
13:03:17 | 14 of 28 START test unique_stg_payments_payment_id................... [RUN]
13:03:17 | 14 of 28 PASS unique_stg_payments_payment_id......................... [PASS in 0.01s]
13:03:17 | 15 of 28 START table model public.customers.......................... [RUN]
13:03:17 | 15 of 28 OK created table model public.customers..................... [SELECT 100 in 0.05s]
13:03:17 | 16 of 28 START table model public.orders............................. [RUN]
13:03:17 | 16 of 28 OK created table model public.orders........................ [SELECT 99 in 0.03s]
13:03:17 | 17 of 28 START test not_null_customers_customer_id................... [RUN]
13:03:17 | 17 of 28 PASS not_null_customers_customer_id......................... [PASS in 0.02s]
13:03:17 | 18 of 28 START test unique_customers_customer_id..................... [RUN]
13:03:17 | 18 of 28 PASS unique_customers_customer_id........................... [PASS in 0.01s]
13:03:17 | 19 of 28 START test accepted_values_orders_status__placed__shipped__completed__return_pending__returned [RUN]
13:03:17 | 19 of 28 PASS accepted_values_orders_status__placed__shipped__completed__return_pending__returned [PASS in 0.02s]
13:03:17 | 20 of 28 START test not_null_orders_amount........................... [RUN]
13:03:17 | 20 of 28 PASS not_null_orders_amount................................. [PASS in 0.01s]
13:03:17 | 21 of 28 START test not_null_orders_bank_transfer_amount............. [RUN]
13:03:17 | 21 of 28 PASS not_null_orders_bank_transfer_amount................... [PASS in 0.01s]
13:03:17 | 22 of 28 START test not_null_orders_coupon_amount.................... [RUN]
13:03:17 | 22 of 28 PASS not_null_orders_coupon_amount.......................... [PASS in 0.01s]
13:03:17 | 23 of 28 START test not_null_orders_credit_card_amount............... [RUN]
13:03:17 | 23 of 28 PASS not_null_orders_credit_card_amount..................... [PASS in 0.01s]
13:03:17 | 24 of 28 START test not_null_orders_customer_id...................... [RUN]
13:03:18 | 24 of 28 PASS not_null_orders_customer_id............................ [PASS in 0.02s]
13:03:18 | 25 of 28 START test not_null_orders_gift_card_amount................. [RUN]
13:03:18 | 25 of 28 PASS not_null_orders_gift_card_amount....................... [PASS in 0.01s]
13:03:18 | 26 of 28 START test not_null_orders_order_id......................... [RUN]
13:03:18 | 26 of 28 PASS not_null_orders_order_id............................... [PASS in 0.01s]
13:03:18 | 27 of 28 START test relationships_orders_customer_id__customer_id__ref_customers_ [RUN]
13:03:18 | 27 of 28 PASS relationships_orders_customer_id__customer_id__ref_customers_ [PASS in 0.02s]
13:03:18 | 28 of 28 START test unique_orders_order_id........................... [RUN]
13:03:18 | 28 of 28 PASS unique_orders_order_id................................. [PASS in 0.01s]
13:03:18 |
13:03:18 | Finished running 3 seeds, 3 view models, 20 tests, 2 table models in 1.00s.

Completed successfully

Done. PASS=28 WARN=0 ERROR=0 SKIP=0 TOTAL=28

@pecigonzalo
Copy link
Author

I see the mistake I made tho, since we did this 0.19 -> 020 and 0.20 -> 0.21 and then we did 0.21 -> 1.0, I assumed we ran the same test on 0.21 -> 1.0, but we did not, and 0.21 -> 1.0 does indeed break.

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 0.20 -> 0.21.

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

@gshank
Copy link
Contributor

gshank commented May 4, 2022

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.

@jtcohen6 jtcohen6 mentioned this issue May 11, 2022
6 tasks
@leahwicz leahwicz added the Refinement Maintainer input needed label May 31, 2022
@jtcohen6
Copy link
Contributor

jtcohen6 commented Jun 8, 2022

Desirable end state: Users with jobs leveraging state:modified do not need to perfectly coordinate upgrades across jobs/environments when upgrading to a new minor version of dbt, UNLESS that new minor version makes changes to manifest.json that are NOT backwards compatible with older versions of manifest.json. In particular, we should aim to provide backwards compatibility for the version immediately previous, to ease the upgrade path for v1.0 → v1.1, v1.1 → v1.2, and so on.

It feels uncontroversial to me that we should support that behavior for manifest v4v5, i.e. for v1.0 → v1.1, because the change was truly so minor.

So, as a way of generalizing that, here's what I'm thinking:

  • We define a new method on the artifact class that (optionally) defines a set of "previous compatible" versions. For instance, the current manifest version is v5. It is backwards compatible with v4, but not with v3 (serialization errors).
  • We add a functional test that runs dbt "latest" against all older manifest versions, for a simple project (generated using dbt v0.19+). We test --state behavior with all older manifest versions, expecting success for "compatible" versions and a clear error message (IncompatibleSchemaException) for "incompatible" versions.

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 manifest.json unless they're strictly necessary. I would REALLY like to find a way to avoid bumping the manifest.json, simply because NodeConfig has a new set of default values, when NodeConfig is already an extensible class that will accept any arbitrary user-configured attribute.

@jtcohen6 jtcohen6 removed the Refinement Maintainer input needed label Jun 8, 2022
@pecigonzalo
Copy link
Author

@jtcohen6 thanks for the writeup/summary.

UNLESS that new minor version makes changes to manifest.json that are backwards compatible with older versions of manifest.json.

This part is a bit unclear. Could you clarify it?

@jtcohen6
Copy link
Contributor

jtcohen6 commented Jun 8, 2022

@pecigonzalo Thanks for reading, and catching me there! I was missing a very important instance of the word NOT. Edited the comment above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
artifacts bug Something isn't working state Stateful selection (state:modified, defer) user docs [docs.getdbt.com] Needs better documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants