Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to full use of tools/resourcedocsgen #4663

Merged
merged 11 commits into from
Jun 5, 2024

Conversation

guineveresaenger
Copy link
Contributor

Description

This pull request migrates functionality from https://github.com/pulumi/registrygen to tools/resourcedocsgen which is more actively maintained, in the aim of retiring registrygen.

Before this change

(to the best of my knowledge)

  • All providers were using registrygen metadata to generate metadata files, these are Pulumi.yaml in the themes/default/datafolder and_index.mdandconfiguration-installation.mdin thethemes/default/content` folder.
  • Additionally, 3rd party providers were using a version check functionality to verify whether packages should be updated (on a cron-based Workflow).
  • All providers are using resourcedocsgen to build API docs. This pull request does not change that.

After this change

  • All providers use resourcedocsgen metadata
  • 3rd party providers use resourcedocsgen pkgversion
  • Both commands are updated to contain the functionality of registrygen at this time, with once exception, see below.

It has been a constant source of confusion to maintainers on which tool to use when, as they're very similar and both come with unused code that looks like it's the real thing (but we use the other tool for that). I have personally wasted entirely too much time on this discrepancy.

After this pull request merges, we should be able to retire pulumi/registrygen.

I recommend reviewing these commits one by one:

  1. Migrate registrygen's metadata command to tools/resourcedocsgen
    Moves the current, stable functionality we've been using to resourcedocsgen. There are functionality and use updates.
  2. Ignore the build resourcedocsgen binary - you can ignore the gitignore
  3. Special-case Pulumi for top-level config files not found This commit sends an error for 3rd party providers if the required docs files cannot be found. This will mostly help identify issues automatically upon first-time publication of a 3rd party provider to the pulumi registry. We continue to return nil for Pulumi providers, so that we continue to use the hard coded config docs files in the registry until we've implemented Generate documentation for provider configuration #4749 (work underway).
  4. Update Go module name and update references
    Follows Go module naming protocol. This is a cleanup from the migration from pulumi/docs.
  5. Use resourcedocsgen for generating package metadata - Use new metadata command in CI
  6. Transport pkgversion check to resourcedocsgen - for 3rd party providers, import the pkgversion check as well
  7. Replace pkgversion check - Use new pkgversion command in CI.

Copy link

Your site preview for commit f33fde0 is ready! 🎉

http://registry--origin-pr-4663-f33fde09.s3-website.us-west-2.amazonaws.com/registry.

Copy link
Member

@iwahbe iwahbe left a comment

Choose a reason for hiding this comment

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

I love this!

tools/resourcedocsgen/cmd/metadata.go Outdated Show resolved Hide resolved
tools/resourcedocsgen/pkg/lookup.go Show resolved Hide resolved
tools/resourcedocsgen/pkg/lookup.go Show resolved Hide resolved
Copy link

github-actions bot commented Jun 4, 2024

Your site preview for commit 64068d9 is ready! 🎉

http://registry--origin-pr-4663-64068d91.s3-website.us-west-2.amazonaws.com/registry.

Copy link
Member

@sean1588 sean1588 left a comment

Choose a reason for hiding this comment

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

This is looking great @guineveresaenger ! Thanks so much for doing this!! I tested out these commands locally and outputs match what registrygen was doing before, so looks like the migration is working as expected! Just one thing to address.

Also thank you for this:

This commit sends an error for 3rd party providers if the required docs files cannot be found.

Comment on lines 27 to 28
cd tools/resourcedocsgen && go run github.com/pulumi/registry/tools/resourcedocsgen \
resourcedocsgen metadata --providerName=${{ env.PROVIDER_SHORT_NAME }} --repoSlug pulumi/pulumi-${{ env.PROVIDER_SHORT_NAME }} --schemaFile="${{ env.PROVIDER_SCHEMA_PATH }}" --version="${{ env.PROVIDER_VERSION }}" --publisher Pulumi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cd tools/resourcedocsgen && go run github.com/pulumi/registry/tools/resourcedocsgen \
resourcedocsgen metadata --providerName=${{ env.PROVIDER_SHORT_NAME }} --repoSlug pulumi/pulumi-${{ env.PROVIDER_SHORT_NAME }} --schemaFile="${{ env.PROVIDER_SCHEMA_PATH }}" --version="${{ env.PROVIDER_VERSION }}" --publisher Pulumi
cd tools/resourcedocsgen && go run github.com/pulumi/registry/tools/resourcedocsgen \
metadata --providerName=${{ env.PROVIDER_SHORT_NAME }} --repoSlug pulumi/pulumi-${{ env.PROVIDER_SHORT_NAME }} --schemaFile="${{ env.PROVIDER_SCHEMA_PATH }}" --version="${{ env.PROVIDER_VERSION }}" --publisher Pulumi

I don't think calling resourcedocsgen here will work since you are doing a go run in the line above. So probably just need to pass it the subcommands and args like you are doing here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh! thank you for this catch!

Copy link
Contributor

@cnunciato cnunciato left a comment

Choose a reason for hiding this comment

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

Aside from Sean's comment, this all looks great to me. Thanks so much @guineveresaenger! I've been confused by the existence of both of these also.

Copy link

github-actions bot commented Jun 4, 2024

Your site preview for commit e8bfa18 is ready! 🎉

http://registry--origin-pr-4663-e8bfa186.s3-website.us-west-2.amazonaws.com/registry.

Copy link

github-actions bot commented Jun 4, 2024

Your site preview for commit 4ba53ce is ready! 🎉

http://registry--origin-pr-4663-4ba53ceb.s3-website.us-west-2.amazonaws.com/registry.

Copy link
Member

@sean1588 sean1588 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@guineveresaenger guineveresaenger merged commit 86756fd into master Jun 5, 2024
3 checks passed
@guineveresaenger guineveresaenger deleted the guin/migrate-metadata-cmd branch June 5, 2024 15:25
Copy link

github-actions bot commented Jun 5, 2024

Site previews for this pull request have been removed. ✨

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.

4 participants