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

chore: Add script to identify broken doc links #237

Merged
merged 5 commits into from
Sep 12, 2022

Conversation

ckadner
Copy link
Member

@ckadner ckadner commented Sep 9, 2022

Motivation

Broken links in documentation are making it harder for new comers to learn about ModelMesh and often it is not easy to find what the correct link should be. For project maintainers it is time consuming to revisit documentation to identify broken links manually and it certainly does not make the best impression on new comers if they click on links that return 404 errors.

Modifications

Add new script scripts/verify_doc_links.py and new Make target check-doc-links to identify broken links in any of the currently 211 links in 43 Markdown files.

Result

$ make check-doc-links
Checked 211 links (144 unique URLs) in 43 Markdown files.

docs/configuration/README.md:68: /docs/configuration/predictors/run-inference -> 404
docs/inference/data-types-mapping.md:5: https://github.com/triton-inference-server/server/blob/main/docs/model_configuration.md#datatypes -> 404
docs/model-formats/advanced-configuration.md:64: https://github.com/triton-inference-server/server/blob/main/docs/model_configuration.md#inputs-and-outputs -> 404
docs/model-formats/advanced-configuration.md:54: https://github.com/triton-inference-server/server/blob/main/docs/model_configuration.md#maximum-batch-size -> 404
docs/model-formats/advanced-configuration.md:48: https://github.com/triton-inference-server/server/blob/main/docs/model_configuration.md#scheduling-and-batching -> 404
docs/predictors/README.md:113: /docs/predictors/inferenceservice-cr.md.md#predictor-status -> 404
docs/runtimes/custom_runtimes.md:246: /docs/runtimes/configuration -> 404

ERROR: Found 7 invalid Markdown links
make: *** [check-doc-links] Error 1

And after fixing the broken links:

$ make check-doc-links
Verified 211 links (142 unique URLs) in 43 Markdown files.

check-doc-links: OK

How does the script work

  • Find all Markdown files in the project
  • Extract all Markdown-style links and plain URLs
  • Create unique list of simplified URLs
  • Use local files to check relative links, use cached HTTP requests for links outside project
  • Run HTTP requests in parallel
  • Retry GitHub rate-limited URLs after (at least) one minute
  • Report broken links with source file and line number

Script pedigree: I contributed several iteration of this script to MLX and KFP-Tekton projects. This one differs in that it does not require any non-standard libraries like for Markdown or requests to be installed into a virtual environment prior to running the script.

Possible future enhancements

Add a new Pull Request builder check to run the check-doc-links frequently and fix broken links as soon as possible.


/cc @njhill @chinhuang007

* Find all Markdown files in the project
* Extract all Markdown-style links and plain URLs
* Create unique list of simplified URLs
* Use local files to check relative links, use cached
  HTTP requests for links outside project
* Run HTTP requests in parallel
* Retry GitHub rate-limited URLs after (at least) one minute
* Report broken links with source file and line number

Signed-off-by: Christian Kadner <ckadner@us.ibm.com>
* Remove inactive docs and docs.dev targets

Signed-off-by: Christian Kadner <ckadner@us.ibm.com>
Signed-off-by: Christian Kadner <ckadner@us.ibm.com>
Signed-off-by: Christian Kadner <ckadner@us.ibm.com>
@ckadner ckadner added documentation Improvements or additions to documentation test testing related bugs and fixes labels Sep 9, 2022
Signed-off-by: Christian Kadner <ckadner@us.ibm.com>
Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

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

Thanks @ckadner this is great! Maybe we should consider also adding it to the main kserve repo. That also has a lot of docs/inks.

@kserve-oss-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ckadner, njhill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ckadner
Copy link
Member Author

ckadner commented Sep 12, 2022

Thanks @ckadner this is great! Maybe we should consider also adding it to the main kserve repo. That also has a lot of docs/inks.

Okay, good idea. Will do.


kserve/kserve#2423
kserve/website#186

@njhill
Copy link
Member

njhill commented Sep 12, 2022

/lgtm

@kserve-oss-bot kserve-oss-bot merged commit ca59907 into kserve:main Sep 12, 2022
hdefazio pushed a commit to hdefazio/modelmesh-serving that referenced this pull request Jan 16, 2025
* Use config/ as the manifests source for ODH operator v2

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* move all changes into an overlay

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* restructure manifests so that kustomize does not need LoadRestrictorNone flag

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* correct label mismatch

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* final modifications

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* remove openvino variable as it is unused

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

---------

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>
hdefazio pushed a commit to hdefazio/modelmesh-serving that referenced this pull request Jan 16, 2025
* Manifest readiness for Operator v2 (kserve#237)

* Use config/ as the manifests source for ODH operator v2

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* move all changes into an overlay

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* restructure manifests so that kustomize does not need LoadRestrictorNone flag

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* correct label mismatch

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* final modifications

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* remove openvino variable as it is unused

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

---------

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

* change image tags to 0.11.1

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

---------

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved documentation Improvements or additions to documentation lgtm test testing related bugs and fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants