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

add config descriptions per user feedback #4448

Merged
merged 17 commits into from
Nov 17, 2023
Merged

Conversation

mirnawong1
Copy link
Contributor

@mirnawong1 mirnawong1 commented Nov 13, 2023

per user feedback during coalesce, the config sections don't explain the differences between resource-type configs vs general configs.

this pr adds a config description to the resource config section and general config section.

@mirnawong1 mirnawong1 requested a review from a team as a code owner November 13, 2023 12:42
Copy link

vercel bot commented Nov 13, 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 Nov 17, 2023 6:43pm

@github-actions github-actions bot added content Improvements or additions to content size: x-small This change will take under 3 hours to fix. Docs team Authored by the Docs team @dbt Labs labels Nov 13, 2023
@github-actions github-actions bot added size: small This change will take 1 to 2 days to address and removed size: x-small This change will take under 3 hours to fix. labels Nov 13, 2023
@mirnawong1 mirnawong1 changed the title Mwong config descriptions add config descriptions per user feedback Nov 13, 2023
Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome use of snippets 💪

Practical distinction

At a high level, I'm not aware of a meaningful distinction between resource-specific and general configs that affects end users and how they use dbt.

Conceptual distinction

The only difference I'm aware of:

  • "Resource-specific configurations" are applicable to only one dbt resource type and "general configurations" are applicable to multiple resource types.

But I don't know a practical difference that affect how users think and act.

Radical alternative

So one radical alternative would be to combine the resource-specific and general configs into a single section and then no additional explanations are even needed.

For seeds, the combined listing for dbt_project.yml might look like this (either give or take the inline comments):

seeds:
  <resource-path>:

    # resource-specific configs only available to seeds
    +quote_columns: true | false
    +column_types: {column_name: datatype}
    +delimiter: <string>

    # general configs also available in other resource types
    +enabled: true | false
    +tags: <string> | [<string>]
    +pre-hook: <sql-statement> | [<sql-statement>]
    +post-hook: <sql-statement> | [<sql-statement>]
    +database: <string>
    +schema: <string>
    +alias: <string>
    +persist_docs: <dict>
    +full_refresh: <boolean>
    +meta: {<dictionary>}
    +grants: {<dictionary>}

History

Here are the pull requests where these separate headings originally appeared for each different resource types:

See also

#3964 has some tables showing which general configs apply to which resources. It does not contain any resource-specific configs.

website/snippets/_config-description-general.md Outdated Show resolved Hide resolved
website/snippets/_config-description-resource.md Outdated Show resolved Hide resolved
@mirnawong1
Copy link
Contributor Author

thank you for your detailed technical comments, i appreciate it. 🙏

I think combing them is a great idea, however i do think doing that is still not explicit enough for users. i like the idea of being more explicit so users have more insight into how they can apply each type of config for their own use case, as oppose to inferring it somehow in the comments of header (currently it's inferred via the header, i think).

I've tried to also make it clearer that the resource-specific config is only available that source using props but it's not working for me so I'm going to investigate more.

@dbeatty10
Copy link
Contributor

I've tried to also make it clearer that the resource-specific config is only available that source using props but it's not working for me so I'm going to investigate more.

The distinctions aren't working for me either. I suspect it's because there isn't really any meaningful distinction between resource-specific configs and general configs.

I'm guessing the reason the two different headers exist is just to make it easier to separate the configs that are unique to that particular resource type. e.g., show which configuration options for the snapshot resource type are different than all the rest of the resource types.

I think combing them is a great idea, however i do think doing that is still not explicit enough for users. i like the idea of being more explicit so users have more insight into how they can apply each type of config for their own use case, as oppose to inferring it somehow in the comments of header (currently it's inferred via the header, i think).

What would be some ways to give insight on how to apply each type of config? Do you think the examples section helps with that?

@mirnawong1 mirnawong1 enabled auto-merge November 17, 2023 18:05
@mirnawong1
Copy link
Contributor Author

will create a separate issue to restructure the config pages

@mirnawong1 mirnawong1 merged commit a3e58b5 into current Nov 17, 2023
3 checks passed
@mirnawong1 mirnawong1 deleted the mwong-config-descriptions branch November 17, 2023 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto update content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs 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.

3 participants