-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Compile assets as part of docs generate #2623
Conversation
Sure, I don't see why not.
Yeah. Check out the |
f5065fd
to
6e4e9db
Compare
core/dbt/config/project.py
Outdated
@@ -401,6 +402,7 @@ def from_project_config( | |||
) | |||
|
|||
docs_paths: List[str] = value_or(cfg.docs_paths, all_source_paths) | |||
asset_paths: List[str] = value_or(cfg.asset_paths, ['assets']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
heads up, this differs from the original ticket that had the default as ['static']
, but I prefer the consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, great tests!
The only question I have is: should this default to ['assets']
, or to []
? Maybe you should have to opt-in to have dbt copy your files around? @jtcohen6 I guess this question is for you
CHANGELOG.md
Outdated
@@ -9,6 +9,7 @@ | |||
- Add more helpful error message for misconfiguration in profiles.yml ([#2569](https://github.com/fishtown-analytics/dbt/issues/2569), [#2627](https://github.com/fishtown-analytics/dbt/pull/2627)) | |||
### Fixes | |||
- Adapter plugins can once again override plugins defined in core ([#2548](https://github.com/fishtown-analytics/dbt/issues/2548), [#2590](https://github.com/fishtown-analytics/dbt/pull/2590)) | |||
- Compile assets as part of docs generate ([#2072](https://github.com/fishtown-analytics/dbt/issues/2072), [#2623](https://github.com/fishtown-analytics/dbt/pull/2623/files#)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix this lol
- Compile assets as part of docs generate ([#2072](https://github.com/fishtown-analytics/dbt/issues/2072), [#2623](https://github.com/fishtown-analytics/dbt/pull/2623/files#)) | |
- Compile assets as part of docs generate ([#2072](https://github.com/fishtown-analytics/dbt/issues/2072), [#2623](https://github.com/fishtown-analytics/dbt/pull/2623)) |
Also @drewbanin — can you add your comments re: whether this should work in dbt Cloud? |
Great point, I think this should be opt-in. The majority of users shouldn't need to worry about an |
I was going to argue the counter point — every thing else has a default directory, so we should be consistent here. But, turns out that |
@drewbanin — assigning you since you have context about the potential security implications of this PR |
Just rebased and pushed :) |
resolves #2072
Description
When a user runs
dbt docs generate
, any files in theirassets
directory (configurable!) will be compiled to thetarget/assets
directory — useful for adding images to documentationExtra validation
I added an image to my own project, at
assets/dbt-logo.png
, and then added it as a description:It totally works 😎
Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.