[go/sdk] Delegate alias computation to the engine #11935
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changelog[uncommitted] (2023-01-25)Bug Fixes
|
Zaid-Ajaj
force-pushed
the
zaid/go-alias-including-root-stack
branch
from
fd8b7f5
to
3ad9a40
Compare
Zaid-Ajaj
force-pushed
the
zaid/go-alias-including-root-stack
branch
from
e217164
to
59fb438
Compare
abhinav
reviewed
Jan 20, 2023
There was a problem hiding this comment.
RE: the question about testing:
Someone with more familiarity with the codebase might have a better answer but besides the integration tests, you may be able to install a fake ResourceMonitorClient, intercept RegisterResource calls, and verify that they contain the right Aliases information.
Zaid-Ajaj
force-pushed
the
zaid/go-alias-including-root-stack
branch
3 times, most recently
from
6fc4d5d
to
4379808
Compare
abhinav
reviewed
Jan 23, 2023
Zaid-Ajaj
force-pushed
the
zaid/go-alias-including-root-stack
branch
from
9a22567
to
b5691ab
Compare
Zaid-Ajaj
force-pushed
the
zaid/go-alias-including-root-stack
branch
from
bc4b608
to
c3cbeb3
Compare
3 tasks
abhinav
approved these changes
Jan 25, 2023
|
bors merge |
bors bot
added a commit
that referenced
this pull request
Jan 26, 2023
11935: [go/sdk] Delegate alias computation to the engine r=Zaid-Ajaj a=Zaid-Ajaj # Description This PR removes the use of `collapseAliases` from the go SDK which used to calculate aliases of resources from `ResourceOptions` and their inherited child aliases (a.k.a. alias explosion) and starts using the new alias specification `pulumirpc.Alias` that is handled by the engine in a language-agnostic manner. This PR removes `aliases: []URNOutput` from `resourceState` because we no longer have to keep track of them in `makeResourceState` but instead calculate them in `prepareResourceInputs`. Fixes #11066 Potentially addresses #11697 ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [ ] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: Zaid Ajaj <zaid.naom@gmail.com>
|
Build failed: |
|
bors retry |
|
Build succeeded: |
bors bot
added a commit
that referenced
this pull request
Feb 6, 2023
11981: Test retype remote component and child using aliases r=justinvp a=Zaid-Ajaj # Description Reproduces #11697 and expected to fail right now. Will rebase on #11935 once merged Fixes #11697 (issue) ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [ ] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: Zaid Ajaj <zaid.naom@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR removes the use of
collapseAliasesfrom the go SDK which used to calculate aliases of resources fromResourceOptionsand their inherited child aliases (a.k.a. alias explosion) and starts using the new alias specificationpulumirpc.Aliasthat is handled by the engine in a language-agnostic manner.This PR removes
aliases: []URNOutputfromresourceStatebecause we no longer have to keep track of them inmakeResourceStatebut instead calculate them inprepareResourceInputs.Fixes #11066
Potentially addresses #11697
Checklist
make changelogand committed thechangelog/pending/<file>documenting my change