Ensure patch version is provided if accessing versioned documentation #612
Conversation
- If complete (including patch) version provided, forward as is - If only upto minor version is provided, redirect to patch zero of that minor version Todo: Consider forwarding to latest patch when only minor version provided
| const minor_versioned_pattern = /^(\d+)\.(\d+)\/(.*)/ | ||
| if (minor_versioned_pattern.test(request.uri)) { | ||
| const patched_uri = request.uri.replace(minor_versioned_pattern, "$1.$2.0/$3"); | ||
| return temp_redirect(patched_uri, callback); |
There was a problem hiding this comment.
Note: Temp redirect is used so the patch version used can be updated later.
There was a problem hiding this comment.
why do we want to update it later? 🤔
There was a problem hiding this comment.
This is in case we could update the code to get the latest patch live and direct to it.
If we can then temp redirect would be good, as the latest patch version for a minor release can change.
If we are happy with always redirecting to the patch zero release. Then regular redirect should do.
terragrunt/modules/release-distribution/lambdas/doc-router/index.js
Outdated
Show resolved
Hide resolved
| return callback(null, request); | ||
| } | ||
| // Include patch version 0 if only minor version is provided | ||
| // Todo("Forward to latest patch version if possible") |
There was a problem hiding this comment.
I'm not sure if we can do this.
Asked in the original issue.
If we can't let's delete this line. 👍
| return callback(null, request); | ||
| } | ||
| // Include patch version 0 if only minor version is provided | ||
| // Todo("Forward to latest patch version if possible") | ||
| const minor_versioned_pattern = /^(\d+)\.(\d+)\/(.*)/ |
|
Looks great 🥳 |
| return callback(null, request); | ||
| } | ||
| // Include patch version 0 if only minor version is provided |
There was a problem hiding this comment.
| // Include patch version 0 if only minor version is provided | |
| // Include patch version 0 if minor version is provided |
|
Applied to dev-doc.rust-lang.org 👍 |
|
But https://dev-doc.rust-lang.org/1.65.0/std/boxed/struct.Box.html it's not working too. |
|
I redeployed the master branch and apparently those URLs are not working in dev 🤔 I need to find a way to test this. 👀 |
|
Ok. Now https://dev-doc.rust-lang.org/1.65.0/std/boxed/struct.Box.html is working (the issue was my chache probably)
I will apply your changes again |
|
I applied your code and https://dev-doc.rust-lang.org/1.65/std/boxed/struct.Box.html isn't working. Do you have any idea? Maybe the regex substitution doesn't work? |
Hi, I checked the request in browser's dev console.
I suspect the pattern has not matched due to the leading Could you try now? p.s: |
|
Deloyed 👍 I left some comments about the regex |
terragrunt/modules/release-distribution/lambdas/doc-router/index.js
Outdated
Show resolved
Hide resolved
terragrunt/modules/release-distribution/lambdas/doc-router/index.js
Outdated
Show resolved
Hide resolved
|
This is something that I'd love to see added to our rust-lang/infra-smoke-tests. It will probably require creating a new |
|
Looks great, I'm testing this one last time just to be sure 😂 EDIT: applied in dev again 👍 I'll wait a few minutes and see if it works. Let me know if it works for you. |
|
One note for the maintainers: I want to merge this with "squash" strategy. Is that fine or do we want to the contributors to force push? |
This is step one in addressing #360
Have modified the router slightly
This should help in redirecting minor version documentation access to (hopefully) valid pages
Todo/Future: Consider forwarding to latest patch when only minor version provided