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

Generate static index html documentation #8615

Merged
merged 5 commits into from
Oct 6, 2023
Merged

Conversation

mescanne
Copy link
Contributor

@mescanne mescanne commented Sep 11, 2023

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

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@mescanne mescanne requested review from a team as code owners September 11, 2023 14:45
@cla-bot cla-bot bot added the cla:yes label Sep 11, 2023
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.13% 🎉

Comparison is base (be94bf1) 86.47% compared to head (e8c8eb2) 86.61%.
Report is 2 commits behind head on main.

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     
Flag Coverage Δ
integration 83.47% <100.00%> (+0.12%) ⬆️
unit 65.09% <25.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
core/dbt/cli/main.py 98.69% <100.00%> (+<0.01%) ⬆️
core/dbt/cli/params.py 100.00% <100.00%> (ø)
core/dbt/task/generate.py 87.50% <100.00%> (+0.92%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mescanne
Copy link
Contributor Author

Note -- I unchecked the "has no interface changes" as it does have a new command line option (--static) which enables some additional behaviour.

I'd be happy to add some text to the documentation i.e. public, markdown. I assume that should be done at a later stage.

@graciegoheen graciegoheen added the ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering label Sep 19, 2023
@mescanne
Copy link
Contributor Author

Hello - I'm glad someone is assigned. Please let me know if there's anything I can do to help this along.

Thanks.

@jtcohen6
Copy link
Contributor

jtcohen6 commented Oct 2, 2023

@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.

@mescanne
Copy link
Contributor Author

mescanne commented Oct 2, 2023

@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.

@MichelleArk MichelleArk requested review from QMalcolm and jtcohen6 and removed request for VersusFacit and martynydbt October 6, 2023 14:53
Copy link
Contributor

@QMalcolm QMalcolm left a 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 🚀

Also cat tax:
IMG_3839

@QMalcolm QMalcolm merged commit bb249d6 into dbt-labs:main Oct 6, 2023
5 checks passed
QMalcolm pushed a commit that referenced this pull request Oct 9, 2023
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-3105] [Feature] Generate static documentation (static_index.html)
4 participants