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

Instruct users to explicitly override view and incremental materializations #703

Closed
jtcohen6 opened this issue May 3, 2024 · 4 comments
Closed

Comments

@jtcohen6
Copy link

jtcohen6 commented May 3, 2024

Hey Elementary team!

Change to materialization search order

We're making a change to how dbt searches for implementations of the "builtin" materializations (view, table, etc). Previously, dbt would (implicitly) prefer adapter-specific implementations of materializations defined in packages over the out-of-the-box implementations within dbt, without the user necessarily knowing. We're changing this to be more explicit and less surprising.

I see that this package reimplements the table and incremental materializations. Starting with latest patch releases of dbt Core v1.6.14 and v1.7.14, users will see a deprecation message like the following:

$ dbt run -s my_table_model
16:41:27  Running with dbt=1.7.14
16:41:27  Registered adapter: snowflake=1.7.3
16:41:27  Found 31 models, 2 operations, 0 sources, 0 exposures, 0 metrics, 1232 macros, 0 groups, 0 semantic models
16:41:27
16:41:31
16:41:31  Running 1 on-run-start hook
16:41:31  1 of 1 START hook: elementary.on-run-start.0 ................................... [RUN]
16:41:31  1 of 1 OK hook: elementary.on-run-start.0 ...................................... [OK in 0.00s]
16:41:31
16:41:31  Concurrency: 8 threads (target='dev')
16:41:31
16:41:31  1 of 1 START sql table model dbt_jcohen_dev.my_table_model ..................... [RUN]
16:41:31  [WARNING]: Installed package 'elementary' is overriding the built-in
materialization 'table'. Overrides of built-in materializations from installed
packages will be deprecated in future versions of dbt. Please refer to https://d
ocs.getdbt.com/reference/global-configs/legacy-behaviors#require_explicit_packag
e_overrides_for_builtin_materializations for detailed documentation and
suggested workarounds.
16:41:33  1 of 1 OK created sql table model dbt_jcohen_dev.my_table_model ................ [SUCCESS 1 in 2.32s]

Proposal

I'd recommend updating the "Quickstart" guide in your documentation & package README, adding a step where users explicitly opt into the materialization overrides by adding the following code to a macros/ file in their root projects:

{% materialization table, adapter='snowflake' %}
  {{ return(elementary.materialization_table_snowflake()) }}
{% endmaterialization %}

{% materialization incremental, adapter='incremental' %}
  {{ return(elementary.materialization_table_snowflake()) }}
{% endmaterialization %}

(Replacing snowflake with bigquery, default, etc as appropriate)

In the meantime, users can opt out of this behavior by setting this flag explicitly in their project files:

# dbt_project.yml
flags:
  require_explicit_package_overrides_for_builtin_materializations: False

They will continue to see the deprecation warning.

Timeline

This flag was added (opt-in, disabled by default) in dbt Core v1.6.14 + v1.7.14.

The default value of this flag will be switching from False to True in dbt Core v1.8.0 (release candidate, final released planned for ~May 8) and dbt Cloud (~May 15).

References

@Maayan-s
Copy link
Collaborator

Maayan-s commented May 5, 2024

Hey @jtcohen6 ,
Thanks for the heads up!

It sounds like a reasonable change.
Out of curiosity, did you get complaints from users about it?
We have thousands of users and very few users ever asked us something about it.

About your proposal -
Changing the documentation will certainly work for new installs of the package.
However, existing users will experience the functionality breaking without a version update of Elementary.
Do you plan to notify about this change in your release notes?
Could you mention explicitly the packages that override materializations for users to know the change is relevant for them?

Do you have any other thoughts on how we can update the existing users?
Ideally, I would want an info message on the console saying Note that package XXXX has it's own version of table materialization. To leverage it, use the flag.... like the info message you added for the 1.6 and 1.7 users.

@haritamar
Copy link
Collaborator

haritamar commented May 5, 2024

Hey, @jtcohen6 ,
Just to add on top of @Maayan-s 's comment above, this change also seems to affect the test materialization, which a significant part of our package's testing capabilities relies on.

I've noticed that you mentioned this in your docs:

In the future, we may extend the project-level [dispatch configuration](https://docs.getdbt.com/reference/project-configs/dispatch-config) to support a list of authorized packages for overriding built-in materialization.

Would you be open to us contributing this feature?
I think this can significantly ease the setup changes required from our users (compared to every user having to override materializations)

@jtcohen6
Copy link
Author

jtcohen6 commented May 5, 2024

@Maayan-s @haritamar Thanks for the quick responses!

Yes, some users pointed out to us that it was surprising to have materializations overridden simply by installing the package, without explicit opt-in. I still want to preserve the possibility of this behavior, but it shouldn't be something that happens implicitly & by default.

I appreciate this will require some action from end users regardless, and I'm open to a change in dbt-core that minimizes the amount of work that users need to do directly.

Would you be open to us contributing this feature?

Yes. I've opened an issue in dbt-core, please comment there:

I'll want to pass this by some of the Core engineers (tomorrow), but once we're aligned on next steps, we can try to include this work as a fast-follow

@haritamar
Copy link
Collaborator

Closing this issue for now, we'll follow up on the issue above.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants