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

Embed SDK version #920

Merged
merged 5 commits into from
May 14, 2024
Merged

Embed SDK version #920

merged 5 commits into from
May 14, 2024

Conversation

danielrbradley
Copy link
Member

  1. Change default config to use push-based Go SDK publishing
  2. Use gopatch to enable RespectSchemaVersion in all languages
  3. Rebuild SDKs if we've updated the resources.go file

Related to #915 and https://github.com/pulumi/home/issues/3372

@danielrbradley danielrbradley requested a review from a team May 10, 2024 16:16
@danielrbradley danielrbradley self-assigned this May 10, 2024
@iwahbe
Copy link
Member

iwahbe commented May 10, 2024

@danielrbradley Can you pick a bridged provider and deploy this code all the way through a release?

@danielrbradley
Copy link
Member Author

danielrbradley commented May 13, 2024

@danielrbradley
Copy link
Member Author

A couple of warts we might want to address that I spotted during the first test rollout:

  • The SDK commit actually contains all the SDKs, not only the Go SDK. This is because we just point the Go SDK publish action as the module - which is sdk/ - where the go.mod lives - which also includes all the other SDKs. This isn't a massive issue, but perhaps we could extend the action to allow globbing to select which files to include/exclude.
  • Linked to the above issue, we also include the go.tar.gz file as we currently always download the assets into the sdk folder rather than a temp directory ... which results in it getting included when we publish the sdk/... go module.

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.

A couple of warts we might want to address that I spotted during the first test rollout:

  • The SDK commit actually contains all the SDKs, not only the Go SDK. This is because we just point the Go SDK publish action as the module - which is sdk/ - where the go.mod lives - which also includes all the other SDKs. This isn't a massive issue, but perhaps we could extend the action to allow globbing to select which files to include/exclude.
  • Linked to the above issue, we also include the go.tar.gz file as we currently always download the assets into the sdk folder rather than a temp directory ... which results in it getting included when we publish the sdk/... go module.

I think it's worth addressing both before we deploy to all providers. I don't want to regress in download size, so we need to address the second point before deploying. For simplicity sake, I would like to address the first point as well, but it's not necessary.

but perhaps we could extend the action to allow globbing to select which files to include/exclude

Instead, can we just hard-code the list of paths we want to include? Right now, any pulumi provider should have all of their go code within sdk/{go.mod,go.sum,go}. pulumi/publish-go-sdk-action@v1 can just know that.

@danielrbradley
Copy link
Member Author

danielrbradley commented May 14, 2024

I think it's worth addressing both before we deploy to all providers. I don't want to regress in download size, so we need to address the second point before deploying. For simplicity sake, I would like to address the first point as well, but it's not necessary.

Done - I make the action customisable with globs for the files to include, then customised the list of files for our use case within ci-mgmt as this might change in the future as we do things like move the go.mod one day or publish to a different repo.

Rolling out a release for the xyz provider to test this last change:

Copy link
Contributor

@VenelinMartinov VenelinMartinov left a comment

Choose a reason for hiding this comment

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

looks sensible

@danielrbradley danielrbradley merged commit c34e73d into master May 14, 2024
5 checks passed
@danielrbradley danielrbradley deleted the embed-sdk-version branch May 14, 2024 12:57
@danielrbradley danielrbradley mentioned this pull request May 15, 2024
78 tasks
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.

3 participants