Skip to content

Conversation

@LeahMarieBush
Copy link
Contributor

@LeahMarieBush LeahMarieBush commented Dec 19, 2024

Description

This PR adds support for passing products to the api/all-docs-paths route. This is necessary so only the paths from product migrated to UDR are used in dev-portal. Now api/all-docs-paths will return all the paths, api/all-docs-paths/terraform will return only terraform paths, and api/all-docs-paths/terraform/ptfe-releases/... will return paths for terraform, ptfe-releases, and any other products after that.

EDIT: also this PR now changes the way that docsPaths data is retrieved. Now there is a script that runs during pre-build that creates a JSON file containing all the paths. The paths are then retrieved from that JSON file when the /all-docs-paths endpoint is hit.

Testing

  1. Go to /api/all-docs-paths
  2. This should show all paths from UDR
  3. Go to /api/all-docs-paths?products=terraform
  4. This should show only paths for the pages in the terraform section
  5. Go to /api/all-docs-paths?products=terraform&products=terraform-docs-common
  6. This should show only paths for the pages in the terraform and terraform-docs-common section

@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2024

Vercel Previews Deployed

Name Status Preview Updated (UTC)
Dev Portal ✅ Ready (Inspect) Visit Preview Jan 7, 2025, 3:43 PM
Unified Docs API ✅ Ready (Inspect) Visit Preview Jan 7, 2025, 3:28 PM

@RubenSandwich
Copy link
Contributor

What I believe is happening:

Because we Vercel deploys it's API endpoints as "Node.js Functions" we have certain limitations. One specifically that I believe we are hitting is the 250mb size limit. In that when you use any Node FS module methods Vercel will try to bundle the required files into the Function so that the Function can run independently of your whole app. This is not possible for us as /content is beyond the 250mb limit.

So to get around that limit we upload our content to the Vercel CDN by writing transformed content and assets out to /public, which Vercel automatically uploads to it's CDN/Edge Network at build time. (You can see this in Vercel in your Deployment Details at Deployment Summary/Static Assets.) Which means that your function does not have the actual "full app" available to it. So if you want to access a file you need to do so through the CDN, which is why our "readFile" function is actually a fetch.

So to get the /all-docs-path api to work reliably during the prebuild action we should build a JSON file that lists out the directory structure of /content and upload and use that JSON file next to /all-docs-path api's route.ts file. (Instead of using any Node FS module methods) As crawling our whole CDN to get all of the paths would be error prone and very slow.

@LeahMarieBush LeahMarieBush marked this pull request as ready for review January 3, 2025 17:15
@LeahMarieBush LeahMarieBush requested a review from a team as a code owner January 3, 2025 17:15
@LeahMarieBush LeahMarieBush requested review from RubenSandwich and prestonbourne and removed request for a team January 3, 2025 17:15
Copy link
Contributor

@RubenSandwich RubenSandwich left a comment

Choose a reason for hiding this comment

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

The API presented here for specifying only specific docs paths breaks URL conventions. In that in URL conventions after a / is a way to denote a child/deeper dive relationship.

e.g.

hashicorp.com/products/terraform
developer.hashicorp.com/packer/docs
developer.hashicorp.com/packer/docs/commands/init

So the API working by tacking on multiple products at the end of a URL path is confusing as not all of our products are linked in child/parent relationship to each other.

e.g.
/api/all-docs-paths/terraform/nomad

makes no sense, as nomad is not a part of terraform.

Rather we should use URL params to specify the change in the API we desire. I suggest something like:

/api/all-docs-paths?only=[terraform,nomad]

This way we keep with URL conventions and make it much easier to understand what the api is actually doing.

Copy link
Contributor

@RubenSandwich RubenSandwich left a comment

Choose a reason for hiding this comment

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

Works and looks great! Thanks for going through 2 rounds of reviews.

@LeahMarieBush LeahMarieBush merged commit f2c5a26 into main Jan 7, 2025
3 checks passed
@LeahMarieBush LeahMarieBush deleted the leah/feat/add-optional-products-to-all-docs-paths branch January 7, 2025 15:47
im2nguyen pushed a commit that referenced this pull request Mar 6, 2025
* feat: optional productSlugs param for all-docs-paths

* add tests and fix type

* generate docsPaths json file and serve data from there

* testing WIP

* add more tests

* PR feedback: change to using url search params

* PR feedback: add another test
hashibot-web added a commit that referenced this pull request Apr 18, 2025
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.

2 participants