-
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
Generate static index html documentation #8615
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #8615 +/- ##
==========================================
+ Coverage 86.47% 86.61% +0.13%
==========================================
Files 174 174
Lines 25597 25603 +6
==========================================
+ Hits 22136 22176 +40
+ Misses 3461 3427 -34
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
conversions on Windows. (similar behaviour as index.html)
Hello - I'm glad someone is assigned. Please let me know if there's anything I can do to help this along. Thanks. |
@mescanne Thanks so much for opening these PRs! It's one thing to weigh in on a highly upvoted issue; it's another to push up the code to make it happen. @QMalcolm and I have taken a first look together. We both appreciate the care you've taken to make this an additive change, with minimal risk for existing functionality. Unfortunately, our planned time to look again together on Friday was interrupted by our need to respond to an emergent regression in v1.6.4 (#8749). Overall, I am supportive of this change. We're coming close to the wire for the things we must get in for dbt Core v1.7, in support of our top-level initiatives this year (Semantic Layer and Model Governance / Multi-project), so we likely won't be able to include this change in 1.7 — but I'd be happy to have it in for the following release. |
@jtcohen6 Thanks for the response. I've struggled with work arounds to this for quite some time, so if it's in the 1.8 mainline that's fine, too. I'm also happy to engage in a discussion about the approach. |
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.
@mescanne Thank you for doing this work! Sorry about the lag time in getting this approved, I adopted a cat and then immediately got sick 🙃 I've been through the code and it looks fantastic! Additionally, I've tried the changes locally, and I didn't run into any unexpected situations. Thanks again, we'll get it shipped 🚀
* Include option to generate static index.html * Added changie * Using DBT's system load / write file methods for better cross platform support * Updated docs tests with dbt.client.systems calls for file reading * Writing out static_index.html as binary file to prevent line-ending conversions on Windows. (similar behaviour as index.html)
resolves #8614
Problem
Generated documentation that depends on external manifest.json and catalog.json must be hosted by a webserver. There are circumstances (such as in a local filesystem or in an object storage) where this creates an extra step for inspecting the documentation that isn't necessary with a static index.html.
Solution
By upgrading the index.html (see dbt-docs PR#465), it allows for generation of static index.html through search-and-replace of the manifest.json and catalog.json data within the index.html.
This is backwards compatible (and indeed produces static_index.html as output) and is a technique that can be easily used across new outputs if the documentation is enhanced in the future.
Checklist