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

Enable resource reference feature by default #5905

Merged
merged 2 commits into from
Dec 10, 2020

Conversation

lblackstone
Copy link
Member

Unless the PULUMI_DISABLE_RESOURCE_REFERENCES flag
is explicitly set to a truthy value, the resource reference feature is now
enabled by default.

Fix #5838

Unless the PULUMI_DISABLE_RESOURCE_REFERENCES flag
is explicitly set to a truthy value, the resource reference feature is now
enabled by default.
@justinvp
Copy link
Member

justinvp commented Dec 9, 2020

@lblackstone, I have a WIP change that turns on resource references end-to-end for Node and Python 06ec7d6. I need to update that commit to do 2 things: 1) instead of hardcoding those to true, only set it to true if PULUMI_EXPERIMENTAL_RESOURCE_REFERENCES is set and 2) also set these in the .NET and Go SDKs. I was hoping we could then kick off another cron run of the examples repo with these changes. And if that looks good, then flip all the places where PULUMI_EXPERIMENTAL_RESOURCE_REFERENCES is used to use PULUMI_DISABLE_RESOURCE_REFERENCES.

@justinvp
Copy link
Member

justinvp commented Dec 9, 2020

And if that looks good, then flip all the places where PULUMI_EXPERIMENTAL_RESOURCE_REFERENCES is used to use PULUMI_DISABLE_RESOURCE_REFERENCES.

I opened #5908 but decided to just skip ahead to turning it on in the SDKs by default with PULUMI_DISABLE_RESOURCE_REFERENCES to opt-out. So that PR now depends on this PR. Wondering if we should just pull the commit from that PR into this PR, so we can merge the changes as a single commit into master? @lblackstone, @pgavlin, what do you think?

@pgavlin
Copy link
Member

pgavlin commented Dec 9, 2020

Wondering if we should just pull the commit from that PR into this PR, so we can merge the changes as a single commit into master? @lblackstone, @pgavlin, what do you think?

That sounds good to me.

This can be disabled by setting the `PULUMI_DISABLE_RESOURCE_REFERENCES` environment variable to a truthy value.
@justinvp justinvp marked this pull request as ready for review December 10, 2020 01:00
Copy link
Member

@justinvp justinvp left a comment

Choose a reason for hiding this comment

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

@lblackstone, your commit LGTM.

@justinvp
Copy link
Member

Is there a good way to get some more validation on this in examples repo, now that it'll be enabled in the SDKs as well? If we merge this in master and kick off the cron job in examples repo, will it use the dev builds of the SDKs?

@lblackstone
Copy link
Member Author

Is there a good way to get some more validation on this in examples repo, now that it'll be enabled in the SDKs as well? If we merge this in master and kick off the cron job in examples repo, will it use the dev builds of the SDKs?

@stack72 Do you know the answer to this?

@pgavlin
Copy link
Member

pgavlin commented Dec 10, 2020

If we merge this in master and kick off the cron job in examples repo, will it use the dev builds of the SDKs?

IIRC the answer is yes.

@lblackstone lblackstone merged commit 4d48ee0 into master Dec 10, 2020
@pulumi-bot pulumi-bot deleted the lblackstone/default-resource-ref branch December 10, 2020 18:21
abhinav pushed a commit to pulumi/pulumi-dotnet that referenced this pull request Jan 11, 2023
* Enable resource reference feature by default

Unless the PULUMI_DISABLE_RESOURCE_REFERENCES flag
is explicitly set to a truthy value, the resource reference feature is now
enabled by default.

* Set AcceptResources in the language SDKs

This can be disabled by setting the `PULUMI_DISABLE_RESOURCE_REFERENCES` environment variable to a truthy value.

Co-authored-by: Justin Van Patten <jvp@justinvp.com>
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.

Turn on support for feature "resourceReferences"
3 participants