Conversation
|
@Bisaloo I gave you maintainer rights on this repo so you should be able to merge when you think it is ready now |
There was a problem hiding this comment.
Pull request overview
This PR introduces stability-focused configuration changes for the Bioconductor code explorer, primarily by tightening crawler access via robots.txt and adding stricter Kubernetes CronJob execution controls to prevent overlapping or runaway jobs.
Changes:
- Update
robots.txtgeneration to block specific bots and restrict default crawling to a limited set of paths. - Add CronJob safeguards (
concurrencyPolicy,startingDeadlineSeconds,activeDeadlineSeconds) to reduce overlap and bound runtime. - Reduce CPU request for the indexing CronJob.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| mirror-updater/utils.R | Adjusts generated robots.txt bot exclusions and allow/disallow rules intended to reduce crawler load. |
| kubernetes/cronjob-logrotate.yaml | Adds CronJob scheduling safeguards to prevent concurrent runs and bound job runtime. |
| kubernetes/cronjob-index.yaml | Adds CronJob scheduling safeguards, bounds runtime, and lowers CPU request for indexing job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| writeLines("User-agent: *", con = con) | ||
|
|
||
| writeLines(paste0("Allow: /browse/*/"), con = con) | ||
| writeLines(paste0("Disallow: /browse/*/*/"), con = con) | ||
| writeLines(paste0("Disallow: /browse/*/treegraph"), con = con) | ||
| writeLines(paste0("Disallow: /browse/themes"), con = con) | ||
| writeLines(paste0("Disallow: *tree/"), con = con) | ||
| writeLines(paste0("Disallow: *blob/"), con = con) | ||
| writeLines(paste0("Disallow: *commit"), con = con) | ||
| writeLines(paste0("Disallow: *stats/"), con = con) | ||
| writeLines(paste0("Disallow: *network/"), con = con) | ||
| writeLines(paste0("Disallow: *RELEASE_"), con = con) | ||
| writeLines(paste0("Disallow: *raw/"), con = con) | ||
| writeLines(paste0("Disallow: *logpatch/"), con = con) | ||
| writeLines(paste0("Disallow: *zipball/"), con = con) | ||
| writeLines(paste0("Disallow: *tarball/"), con = con) | ||
| writeLines(paste0("Disallow: *blame/"), con = con) | ||
| writeLines(paste0("Disallow: *rss/"), con = con) | ||
|
|
||
| writeLines(paste0("Disallow: /search/search?q"), con = con) | ||
| writeLines("Disallow: /", con = con) | ||
| writeLines("Allow: /browse/", con = con) | ||
| writeLines("Allow: /search/", con = con) | ||
| writeLines("Allow: /index.html", con = con) | ||
| writeLines("Allow: /about.html", con = con) | ||
| writeLines(sprintf("Allow: /browse/%s/", basename(pkgs)), con = con) |
There was a problem hiding this comment.
In the User-agent: * block, Allow: /browse/ is a prefix match and effectively permits crawling of the entire browse subtree (including /browse/<repo>/blob/..., commits, etc.). If the goal is to reduce crawler load for stability, consider narrowing the allow rules (e.g., only the repo landing pages) and/or reintroducing explicit disallows for the expensive browse endpoints.
|
|
||
| writeLines(paste0("Disallow: /search/search?q"), con = con) | ||
| writeLines("Disallow: /", con = con) | ||
| writeLines("Allow: /browse/", con = con) |
There was a problem hiding this comment.
Allow: /search/ will also allow crawling of the results endpoint /search/search?... (the results template links to search?q=..., which resolves under /search/). To avoid crawlers generating large query loads, add a more specific Disallow: /search/search (and optionally other query-style paths) while still allowing the search landing page.
| writeLines("Allow: /browse/", con = con) | |
| writeLines("Allow: /browse/", con = con) | |
| writeLines("Disallow: /search/search", con = con) |
| "AwarioRssBot", "AwarioSmartBot", "ImagesiftBot", | ||
| "AliyunSecBot", "Aliyun", "Bytespider") | ||
| "AliyunSecBot", "Aliyun", "Bytespider", "YandexBot", | ||
| "AcademicBotRTU", "Claude-SearchBot", "dotbot", |
There was a problem hiding this comment.
excluded_bots includes "dotbot", but the commonly used user-agent string is DotBot. Robots user-agent matching isn’t consistently case-sensitive across crawlers/implementations; to ensure the rule applies, consider using the canonical casing and/or including both variants.
| "AcademicBotRTU", "Claude-SearchBot", "dotbot", | |
| "AcademicBotRTU", "Claude-SearchBot", "DotBot", "dotbot", |
| jobTemplate: | ||
| spec: | ||
| ttlSecondsAfterFinished: 100 | ||
| activeDeadlineSeconds: 780 |
There was a problem hiding this comment.
activeDeadlineSeconds: 780 will hard-kill the job if indexing runs longer than ~13 minutes. The mirror-updater scripts create a lock file and only clear it at normal exit; if the pod is terminated by the deadline, the stale lock will cause subsequent runs to exit for up to 18 hours (see createLockFile() behavior). Consider removing/raising the active deadline, and/or adding a shutdown trap in the container entrypoint to delete the lock file on SIGTERM.
| activeDeadlineSeconds: 780 |
No description provided.