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-2788] [Bug] When warnings about contract disenforcement are created, ambiguity is introduced by the passive voice #8029

Closed
2 tasks done
joellabes opened this issue Jul 5, 2023 · 2 comments
Labels
bug Something isn't working model_contracts

Comments

@joellabes
Copy link
Contributor

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

Sibling to #8028

Another warning that dbt emits is:

Breaking Change to Contract Error in model model_x (model_x.sql)
  While comparing to previous project state, dbt detected a breaking change to an enforced contract.
  
  The contract's enforcement has been disabled.

When I first read this, I interpreted it as "dbt detected a breaking change to an enforced contract, so dbt has stopped enforcing the contract" (which is the exact opposite of what I would want; that's exactly when dbt needs to step up!). In fact, it is "dbt detected that the user disabled contract enforcement, which means the contract is now broken."

I think this would be less confusing if I happened to see it as part of a list of issues, but my first encounter was this single bullet item.

Expected Behavior

Maybe adding a - at the start to show it's a list would be enough?
Maybe rephrasing it: Enforcement of the contract was disabled in path/to/the/file.yml?

I don't know exactly

Steps To Reproduce

  • Create a model with a contract which is enforced
  • Run dbt compile
  • Disable the contract
  • Run dbt run -s state:modified --state path/to/artifacts

Relevant log output

No response

Environment

- OS:
- Python:
- dbt:

Which database adapter are you using with dbt?

No response

Additional Context

No response

@joellabes joellabes added bug Something isn't working triage labels Jul 5, 2023
@github-actions github-actions bot changed the title [Bug] When warnings about contract disenforcement are created, ambiguity is introduced by the passive voice [CT-2788] [Bug] When warnings about contract disenforcement are created, ambiguity is introduced by the passive voice Jul 5, 2023
@jtcohen6
Copy link
Contributor

jtcohen6 commented Jul 5, 2023

@joellabes This is an issue after my heart! And my high-school English teacher who penalized me every time I used the passive voice. "'Mistakes were made.' Leave the passive voice to the politicians, and the Times editorial board."

I think this would be less confusing if I happened to see it as part of a list of issues, but my first encounter was this single bullet item.

I think this is exactly what's at play: If you disable a model's contract and make some breaking changes to its content, the idea is to show all of those things in one error message. That feels important because we expect many users to run into this error at CI time, where they don't have access to the same quick iteration loop as dev (although we'll still only be able to report the Breaking Changes error for one contracted model at a time).

$ dbt -q ls -s state:modified --state state/
10:00:32  Encountered an error:
Breaking Change to Contract Error in model my_model (models/my_model.sql)
  While comparing to previous project state, dbt detected a breaking change to an enforced contract.

  The contract's enforcement has been disabled.

  Columns were removed:
   - this_column_will_be_removed

  Columns with data_type changes:
   - id (integer -> text)

  Consider making an additive (non-breaking) change instead, if possible.
  Otherwise, create a new model version: https://docs.getdbt.com/docs/collaborate/govern/model-versions

So, potential changes:

  • Reword this sentence to be much more explicit: "Previously, this model's configuration included contract: {enforced: true}. It is no longer configured to enforce its contract, and this is a breaking change."
  • Make the formatting of the error message more like a list:
While comparing to previous project state, dbt detected one or more breaking changes to a model with an enforced contract.
  - Contract enforcement was removed: Previously, this model's configuration included contract: {enforced: true}. It is no longer configured to enforce its contract, and this is a breaking change.
  - Columns were removed:
     - this_column_will_be_removed
  - Columns with data_type changes:
     - id (integer -> text)

@jtcohen6
Copy link
Contributor

jtcohen6 commented Aug 2, 2023

@jtcohen6 jtcohen6 closed this as completed Aug 2, 2023
@jtcohen6 jtcohen6 removed this from the v1.6.x milestone Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working model_contracts
Projects
None yet
Development

No branches or pull requests

3 participants