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

Display supported incremental strategies in a tabular format #4649

Merged
merged 5 commits into from
Dec 14, 2023

Conversation

dbeatty10
Copy link
Contributor

@dbeatty10 dbeatty10 commented Dec 14, 2023

Preview

resolves #4634

What are you changing in this pull request and why?

The main aim is to be able to easily answer each of these questions:

  • which adapters support the delete+insert strategy?
  • what are the names of the standard incremental strategies that are supported in at least one adapter?
  • what are the names of the standard incremental strategies that are supported most frequently?
  • what are the names of the standard incremental strategies that are supported least frequently?

A tabular representation with one column per strategy is the easiest way to answer all of the above.

Design decisions to be made

  1. Style of the icons
  2. Number of tables

Style of the icons

Any of these could work:
✅ - Green check mark
✔️ - Heavy check mark
🟢 - Green circle

We probably wouldn't choose any of these:
☑️ - Ballot box with check
🆗 - OK button
👍 - Thumbs up
etc.

Number of tables

We have several options:

  1. Keep the status quo
  2. Split into two tables: one for the available strategies, and another for the defaults
  3. Keep a single table, keeping the "default strategy" column, but replacing the "additional supported strategies" with one column per strategy
  4. Defer the default strategy information to each adapter-specific config page instead (see dbt-bigquery or dbt-redshift for examples)

Trade-offs

I strongly prefer not staying with the status quo, or I wouldn't have opened this issue 🤣

Splitting into two separate tables would solve the key problem, but at the cost of taking up more vertical space on the page.

Keeping a single table but refactoring it would work for me.

Completely moving the default strategy information into the adapter-specific pages might be the best of all worlds. It would solve the problem at hand while preserving documentation about the default strategy that is relevant for a particular adapter. dbt-postgres and dbt-redshift in particular have caveats related to the default -- it will change depending if a unique_key is defined or not.

🎩

Preview

v3 (final)

image image

v2

image

v1

Note: I've since removed the 2nd table that has the defaults, but here's what it looked like with them:

image image

Checklist

Copy link

vercel bot commented Dec 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-getdbt-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 14, 2023 8:15pm

@github-actions github-actions bot added content Improvements or additions to content size: medium This change will take up to a week to address labels Dec 14, 2023
@github-actions github-actions bot added size: small This change will take 1 to 2 days to address and removed size: medium This change will take up to a week to address labels Dec 14, 2023
@dbeatty10 dbeatty10 marked this pull request as ready for review December 14, 2023 15:06
@dbeatty10 dbeatty10 requested a review from a team as a code owner December 14, 2023 15:06
@matthewshaver
Copy link
Contributor

@dbeatty10 Thanks for putting this together. I like the changes you've made here. I'm going to update the tables to use the ✅ icon. It looks good on light mode, but dark mode users may have trouble seeing the gray on black background.

@dbeatty10 dbeatty10 merged commit 3ad19b4 into current Dec 14, 2023
7 checks passed
@dbeatty10 dbeatty10 deleted the dbeatty/tabular-incremental-strategies branch December 14, 2023 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Improvements or additions to content size: small This change will take 1 to 2 days to address
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display supported incremental strategies in a tabular format for legibility
2 participants