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

sdk/go: Don't drop Dependencies in ResourceOptions snapshot for MLCs #12683

Merged
merged 1 commit into from Apr 18, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Apr 14, 2023

We recently added a new API, NewResourceOptions to build a preview
of a set of resource options.

func NewResourceOptions(...ResourceOption) (*ResourceOptions, error)

This currently drops the dependencies of an MLC from the snapshot.
The reason for this is that it used a special type "urnSet"
to represent dependencies for MLCs received over the wire.
This decision was made at the time because the original Resource objects
were not available for these dependencies.

Turns out that that limitation isn't a blocker:
we can use newDependencyResource to create dummy Resource objects
that hold nothing but a URN.

This allows NewResourceOptions to work on options even inside an MLC.
Note that this currently only works for some of the options:
those that are propagated from construct into the Go SDK options.
Others will be added as part of #12154.

Testing:
The accompanying test failed for Dependencies without this change.

Copy link
Contributor Author

abhinav commented Apr 14, 2023

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Apr 14, 2023

Changelog

[uncommitted] (2023-04-18)

Bug Fixes

  • [sdk/go] Fixed NewResourceOptions dropping MLC dependencies from the options preview.
    #12683

Copy link
Contributor

@dixler dixler left a comment

Choose a reason for hiding this comment

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

This seems fine as it deletes more code than it adds and adds a test.

We recently added a new API, NewResourceOptions to build a preview
of a set of resource options.

    func NewResourceOptions(...ResourceOption) (*ResourceOptions, error)

This currently drops the dependencies of an MLC from the snapshot.
The reason for this is that it used a special type "urnSet"
to represent dependencies for MLCs received over the wire.
This decision was made at the time because the original Resource objects
were not available for these dependencies.

Turns out that that limitation isn't a blocker:
we can use newDependencyResource to create dummy Resource objects
that hold nothing but a URN.

This allows NewResourceOptions to work on options even inside an MLC.
Note that this currently only works for some of the options:
those that are propagated from `construct` into the Go SDK options.
Others will be added as part of #12154.

Testing:
The accompanying test failed for Dependencies without this change.
@abhinav
Copy link
Contributor Author

abhinav commented Apr 18, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Apr 18, 2023

Build succeeded:

@bors bors bot merged commit 5f75dd1 into master Apr 18, 2023
43 of 48 checks passed
@bors bors bot deleted the abhinav/go-snapshot-mlc-deps branch April 18, 2023 21:08
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.

None yet

3 participants