Repository navigation
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test bdb3299 |
|
/ok to test f87a871 |
|
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
f87a871 to
9a24782
Compare
|
/ok to test 9a24782 |
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
|
/ok to test |
📝 Summary
Priority: ⬇️ Low Change: Feature Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
1993dd35-a388-4bbb-8736-2bd9cca57c52
📒 Files selected for processing: 17
.github/actions/check-doc-links/action.yml.github/dependabot.yml.github/workflows/build-docs.yml.github/workflows/lychee.yml.github/workflows/pre-commit.yml.gitignore.pre-commit-config.yamlci/README.mdci/tools/lychee_cache.pyci/tools/retry_lychee.pyci/tools/run_lychee.pyci/tools/run_lychee_precommit.pyci/tools/tests/test_lychee_cache.pyci/tools/tests/test_retry_lychee.pyci/tools/tests/test_run_lychee.pyci/tools/tests/test_run_lychee_precommit.pylychee.toml
💤 Files with no reviewable changes: 2
- .github/dependabot.yml
- .github/actions/check-doc-links/action.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| cache = tmp_path / "rendered cache" | ||
| monkeypatch.setenv("LYCHEE_CACHE_DIR", str(cache)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
suggestion: The expected cache path in this test is not resolved, so the test can fail on macOS and some Windows hosts.
run_lychee.main resolves LYCHEE_CACHE_DIR with Path(...).resolve() (run_lychee.py Line 41). This resolved path is used as cwd and as the parent of inputs.txt. The test compares both values against the unresolved tmp_path / "rendered cache". On macOS, tmp_path sits under the /var → /private/var symlink, so resolve() returns a different path. In that case the run.assert_called_once_with(...) check fails, and --files-from gets a different string than the test expects. Linux CI does not show the problem, but local runs on macOS fail. The fix is to resolve the expected path in the test.
--- "a/ci/tools/tests/test_run_lychee.py"
+++ "b/ci/tools/tests/test_run_lychee.py"
@@ -51,8 +51,8 @@
(docs / "guide.html").write_text('<h1 id="section">Guide</h1>')
(docs / "_static").mkdir()
(docs / "_static/theme.html").write_text("ignored theme")
- cache = tmp_path / "rendered cache"
+ cache = (tmp_path / "rendered cache").resolve()
monkeypatch.setenv("LYCHEE_CACHE_DIR", str(cache))
monkeypatch.setattr(run_lychee, "ROOT", tmp_path)
monkeypatch.delenv("LYCHEE_REPORT_DIR", raising=False)
with (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cache = tmp_path / "rendered cache" | |
| monkeypatch.setenv("LYCHEE_CACHE_DIR", str(cache)) | |
| cache = (tmp_path / "rendered cache").resolve() | |
| monkeypatch.setenv("LYCHEE_CACHE_DIR", str(cache)) |
Highlights
Bring documentation Lychee checks into the same pre-commit workflow used during development, and catch authored-link failures during ordinary PR updates.
lychee-bin==0.24.2dependency. Remove the separate Lychee action, its binary pin, and the obsolete upstream-hook Dependabot exception.main) avoid repeating requests for established external links.Tradeoffs
Authored links are checked more frequently because pre-commit runs on pushes that may not trigger full CI. Cache reuse should keep the usual overhead modest, but new URLs, expired or missing caches, and slow or failing sites still cost requests and time. Each pre-commit job can make up to three Lychee warm-up passes, followed by one strict full pre-commit run; successful external checks are reused between passes. PRs consume the shared baseline without publishing their own snapshots, so newly introduced URLs can be checked again on later pushes.
Successful external checks expire after three days rather than one, providing breathing room when nightly refreshes fail or are missed. Nightly still checks links afresh every day. When fresh results are unavailable, the tradeoff is that a cached success can conceal a newly broken external link for up to 72 hours.
Authored and rendered URL caches remain separate even when their URLs overlap. Authored checks leave fragments unchecked; rendered checks validate them. Lychee's cache does not record which validation occurred, so sharing an authored success can falsely accept a missing anchor. This was reproduced with pinned Lychee 0.24.2 against a local server. Keeping the caches separate prevents that false pass, at the cost of some duplicate requests.
How it works
Repository URLs retain their existing targets in this PR. Checkout-aware repository links are a separate follow-up in #3143. Standalone CI-tool unit tests remain in nightly CI.
Implementation details and suggested review order
Retries and diagnostics
The CI driver invokes the actual configured hook and collects every native invocation's JSON report. Pre-commit may split long input lists, so each invocation writes its own report and exit status before the driver validates and combines them.
Additional hook attempts run only when every remaining failure is recognized as transient HTTP trouble: timeouts, 408, 429, or 5xx. CI allows three total attempts, with 60/120-second cooldowns. Permanent failures stop additional attempts. The full pre-commit suite runs only once, so formatter changes remain failures.
Missing, malformed, or inconsistent reports fail explicitly, as does a combined scan that checks no links. Per-attempt reports and a final summary are retained as CI artifacts for seven days.
Shared caches
Nightly supplies fresh authored and rendered baselines; Linux runs on the default branch also publish authored results. Only successful external checks are persisted, with their original timestamps and a three-day expiration. Reusing a result does not extend its lifetime. Local filesystem targets are checked afresh. A missing baseline starts a normal cold check.
URL results can be reused across operating systems. Installed pre-commit environments retain their separate OS/Python-specific caches. A shared helper derives compatible URL-cache names from the hook configuration, native wheel pin, checker configuration, and implementation files. Hashing whole files is conservative: even a comment or display-name change in a listed file can invalidate the cache.
Suggested review order
The CI link-checking guide collects the maintenance and failure policies in one place. Focused tests cover report aggregation, retry decisions, cache naming, and rendered invocation.
Validation
All-files pre-commit passes locally on both stack layers after the final cache-policy and maintenance-comment update at d7d7bd8f3af2. The stack is based on
mainat 576ef68874f2.Full CI was requested for the preceding revision, 9a247823b2f, before that final update.
Earlier validation of this implementation
Before this rebase and the final review refinements:
Checklist