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

Support remote components in Python #5375

Merged
merged 2 commits into from
Sep 30, 2020
Merged

Support remote components in Python #5375

merged 2 commits into from
Sep 30, 2020

Conversation

justinvp
Copy link
Member

@justinvp justinvp commented Sep 15, 2020

Fixes #5354

@justinvp justinvp force-pushed the justin/py_construct branch 2 times, most recently from 1f00c2d to c48aab3 Compare September 25, 2020 14:21
// a package.json being emitted in the project directory and `yarn install && yarn link @pulumi/pulumi`
// being run.
// When the underlying issue has been fixed, the use of this environment variable should be removed.
if os.Getenv("PULUMI_TEST_YARN_LINK_PULUMI") != "" {
Copy link
Member Author

Choose a reason for hiding this comment

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

Went with an env variable to temporarily enable this, instead of exposing an actual option on ProgramTestOptions, since this is just a temporary workaround.

Originally, I tried doing this as part of building the tests in the Makefile after:

pulumi/Makefile

Lines 71 to 72 in b4c2a35

test_build:: $(SUB_PROJECTS:%=%_install)
cd tests/integration/construct_component/testcomponent && yarn install && yarn link @pulumi/pulumi && yarn run tsc

But our beloved CopyFile utility function that we use to copy the test project dir to a temp dir, doesn't handle copying the symlinks that get created inside the node_modules dir when yarn linking @pulumi/pulumi.

remote: jspb.Message.getBooleanFieldWithDefault(msg, 21, false)
remote: jspb.Message.getBooleanFieldWithDefault(msg, 20, false)
Copy link
Member Author

Choose a reason for hiding this comment

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

I regenerated the protobuf files because they were stale after changing the remote field from 21 to 20 per https://github.com/pulumi/pulumi/pull/5280/files/9ff12da5cc6477e8f3b4ed17115b1d9bde1e9c39#r483125014

Comment on lines +388 to +392
case "child-b":
assert.Equal(t, []resource.URN{urns["a"]}, res.PropertyDependencies["echo"])
case "child-c":
assert.ElementsMatch(t, []resource.URN{urns["child-a"], urns["a"]},
res.PropertyDependencies["echo"])
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: These cases are slightly different than the Node test, because our Node.js SDK filters out ComponentResources from dependencies, but Python does not.

async function getAllTransitivelyReferencedCustomResourceURNs(resources: Set<Resource>) {
// Go through 'resources', but transitively walk through **Component** resources, collecting any
// of their child resources. This way, a Component acts as an aggregation really of all the
// reachable custom resources it parents. This walking will transitively walk through other
// child ComponentResources, but will stop when it hits custom resources. in other words, if we
// had:
//
// Comp1
// / \
// Cust1 Comp2
// / \
// Cust2 Cust3
// /
// Cust4
//
// Then the transitively reachable custom resources of Comp1 will be [Cust1, Cust2, Cust3]. It
// will *not* include `Cust4`.
// To do this, first we just get the transitively reachable set of resources (not diving
// into custom resources). In the above picture, if we start with 'Comp1', this will be
// [Comp1, Cust1, Comp2, Cust2, Cust3]
const transitivelyReachableResources = await getTransitivelyReferencedChildResourcesOfComponentResources(resources);
const transitivelyReachableCustomResources = [...transitivelyReachableResources].filter(r => CustomResource.isInstance(r));
const promises = transitivelyReachableCustomResources.map(r => r.urn.promise());
const urns = await Promise.all(promises);
return new Set<string>(urns);
}

So for Python, we need to include the component resources here.

@justinvp justinvp marked this pull request as ready for review September 25, 2020 15:08
Setting an env variable was affecting other tests, so using a different way to temporarily workaround the dynamic provider issue.
@justinvp justinvp merged commit 9bcf02e into master Sep 30, 2020
@pulumi-bot pulumi-bot deleted the justin/py_construct branch September 30, 2020 21:09
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.

Support remote components in Python
3 participants