Skip to content

link checker - fails PR if links are broken#130

Merged
elevran merged 9 commits intomainfrom
mkdn-link-checker1
May 27, 2025
Merged

link checker - fails PR if links are broken#130
elevran merged 9 commits intomainfrom
mkdn-link-checker1

Conversation

@clubanderson
Copy link
Contributor

@clubanderson clubanderson commented May 21, 2025

Fix #140

@clubanderson clubanderson requested review from elevran and vMaroon May 21, 2025 00:24
Copy link
Collaborator

Choose a reason for hiding this comment

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

The run failed with 4 broken links:

...
🔍 Scanning all Markdown files for broken links...
------------------------------------------------------------
📄 Checking: ./README.md

  ERROR: 3 dead links found in ./README.md !
  [✖] /docs/architecture.md → Status: 400
  [✖] /docs/dp.md → Status: 400
  [✖] /DEVELOPMENT.md → Status: 400
❌ 3 broken links in ./README.md
------------------------------------------------------------
📄 Checking: ./DEVELOPMENT.md

  ERROR: 1 dead links found in ./DEVELOPMENT.md !
  [✖] https://www.gnu.org/software/make/ → Status: 0
❌ 1 broken links in ./DEVELOPMENT.md
------------------------------------------------------------
📄 Checking: ./docs/architecture.md

✅ No broken links in ./docs/architecture.md
------------------------------------------------------------
📄 Checking: ./docs/dp.md

✅ No broken links in ./docs/dp.md
------------------------------------------------------------
📄 Checking: ./docs/create_new_filter.md

✅ No broken links in ./docs/create_new_filter.md
------------------------------------------------------------
❌ Total broken links found: 4
Error: Process completed with exit code 1.

However, upon inspection (and actually clicking on the link...) - all are valid.
We can not merge this PR as it clearly produces false positives. The 400 error code is Bad Request which suggests that the tool is faulty or misconfigured for our environment.

Copy link
Collaborator

@elevran elevran left a comment

Choose a reason for hiding this comment

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

@clubanderson thanks!
this is a good check to have and would love to have it integrated. However, the tool added in this PR currently produces "false positives" and we can not merge it.

@elevran elevran added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels May 21, 2025
@elevran
Copy link
Collaborator

elevran commented May 21, 2025

For the markdown failures in the README.md, it seems that the tool is handling links that GH (markdown and as a web server) process correctly.
Links to files in the local repository can be encoded using an absolute path (e.g., /DEVELOPMENT.md, and GH would interpret them based on the repo's entry point (e.g., https://github.com/llm-d/llm-d-inference-scheduler in our case). The tool seems to treat them as pure absolute path (relative to https://github.com?) and that results in the 400 response.

I'm not sure why it fails on GNU make, though. Note that the return Status code reported for the HTTP request is 0, which is not a valid 3-digit return code.

@elevran elevran self-assigned this May 22, 2025
elevran added 8 commits May 25, 2025 20:59
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
…outs

Signed-off-by: Etai Lev Ran <elevran@gmail.com>
@elevran
Copy link
Collaborator

elevran commented May 25, 2025

@clubanderson - I've made changes needed to your PR to make it pass.
PTAL and let me know what you think

@kfirtoledo kfirtoledo self-requested a review May 27, 2025 06:17
@elevran elevran merged commit 70afea8 into main May 27, 2025
2 checks passed
@elevran elevran deleted the mkdn-link-checker1 branch May 27, 2025 14:51
shmuelk pushed a commit to shmuelk/llm-d-inference-scheduler that referenced this pull request May 29, 2025
* link checker - fails PR if links are broken

Signed-off-by: Andy Anderson <andy@clubanderson.com>

* replace markdown-link-check tool with lychee
* remove markdown-link-check tool custom action
* make README links relative
* increase timeouts and ignore gnu's website since it throttles by timeouts

Signed-off-by: Etai Lev Ran <elevran@gmail.com>

---------

Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Co-authored-by: Etai Lev Ran <elevran@gmail.com>
shmuelk added a commit that referenced this pull request May 29, 2025
* Removed unused make targets and set default tag to dev

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Changed kind development deployer to use dev tag for EPP and Simulator

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Updated documentation to reflect changes

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Removed local change

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Use correct variable in the Makefile

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* Mention the vLLM simulator in the kind environment documentation

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* update GIE upstream version + code adaptations (#135)

* update GIE upstream version + code adaptations

Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>

* addressed code review comments

Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>

---------

Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>

* test: Improve scheduler test (#139)

* Check headers, more tests for pd scheduler

Signed-off-by: Ira <IRAR@il.ibm.com>

* Moved config and context out of the loop, added logger to the context

Signed-off-by: Ira <IRAR@il.ibm.com>

* Removed comment and debug prints

Signed-off-by: Ira <IRAR@il.ibm.com>

---------

Signed-off-by: Ira <IRAR@il.ibm.com>

* Wait for storage pods to exist before moving on (#138)

Signed-off-by: David Martin <davmarti@redhat.com>

* dependabot configuration for Go, GH acions and Docker

Signed-off-by: Etai Lev Ran <elevran@gmail.com>

* link checker - fails PR if links are broken (#130)

* link checker - fails PR if links are broken

Signed-off-by: Andy Anderson <andy@clubanderson.com>

* replace markdown-link-check tool with lychee
* remove markdown-link-check tool custom action
* make README links relative
* increase timeouts and ignore gnu's website since it throttles by timeouts

Signed-off-by: Etai Lev Ran <elevran@gmail.com>

---------

Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Co-authored-by: Etai Lev Ran <elevran@gmail.com>

* Corrected name of vLLM simulator

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

---------

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>
Signed-off-by: Ira <IRAR@il.ibm.com>
Signed-off-by: David Martin <davmarti@redhat.com>
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Co-authored-by: Nir Rozenbaum <nirro@il.ibm.com>
Co-authored-by: Ira Rosen <irar@il.ibm.com>
Co-authored-by: David Martin <david-martin@users.noreply.github.com>
Co-authored-by: Etai Lev Ran <elevran@gmail.com>
Co-authored-by: Andy Anderson <andy@clubanderson.com>
Jooho referenced this pull request in Jooho/llm-d-inference-scheduler Sep 17, 2025
Signed-off-by: red-hat-konflux <126015336+red-hat-konflux[bot]@users.noreply.github.com>
Co-authored-by: red-hat-konflux[bot] <126015336+red-hat-konflux[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable link validity checks in PRs

3 participants