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

core: fix provider/close provider race when targeting #2527

Merged
merged 5 commits into from
Jun 29, 2015

Conversation

phinze
Copy link
Contributor

@phinze phinze commented Jun 26, 2015

When targeting prunes out all the resource nodes between a provider and
its close node, there was no dependency to ensure the close happened
after the configure. Needed to add an explicit dependency from the close
to the provider.

fixes #2495

@phinze
Copy link
Contributor Author

phinze commented Jun 26, 2015

I'll follow up with a similar fix for provisioners

@mitchellh
Copy link
Contributor

It'd be interesting to see how this interact with provider aliases. It might work fine, but its still probably worth adding a test case for them.

@phinze
Copy link
Contributor Author

phinze commented Jun 26, 2015

In addition to test outputs needing updates - this breaks the DisableProviderTransformer as well since that's looking for providers without descendents... looking into it...

When targeting prunes out all the resource nodes between a provider and
its close node, there was no dependency to ensure the close happened
after the configure. Needed to add an explicit dependency from the close
to the provider.

This tweak highlighted the fact that CloseProviderTransformer needed to
happen after DisableProviderTransformer, since
DisableProviderTransformer inspects up-edges to decide what to disable,
and CloseProviderTransformer adds an up-edge.

fixes #2495
@phinze phinze force-pushed the b-close-provider-target branch from dce1772 to 2d6a8c1 Compare June 29, 2015 16:22
Not sure if this test has value /cc @mitchellh (who requested one be
added) to see what I might be missing here.

refs #2495
@mitchellh
Copy link
Contributor

LGTM. You're right that that test probably isn't testing anything. I have an idea, going to hijack your branch for a second and see something.

@phinze
Copy link
Contributor Author

phinze commented Jun 29, 2015

Hijack away! 🚀

@phinze phinze force-pushed the b-close-provider-target branch from 2dc5e9b to 184edbe Compare June 29, 2015 17:46
@phinze
Copy link
Contributor Author

phinze commented Jun 29, 2015

Okay @mitchellh one more review - I think this is G2G.

@mitchellh
Copy link
Contributor

Yep yep LGTM

phinze added a commit that referenced this pull request Jun 29, 2015
core: fix provider/close provider race when targeting
@phinze phinze merged commit 3060050 into master Jun 29, 2015
@phinze phinze deleted the b-close-provider-target branch June 29, 2015 18:34
@svanharmelen
Copy link
Contributor

👍

@ghost
Copy link

ghost commented May 1, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Templates not working with latest master branch
4 participants