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

Change invoke call to always use latest version #987

Merged
merged 4 commits into from
Feb 11, 2020

Conversation

lblackstone
Copy link
Member

Proposed changes

Related issues (optional)

} else {
invokeOpts = {...invokeOpts, version: getVersion()};
}
let invokeOpts: pulumi.InvokeOptions = { async: true, version: getVersion() };
Copy link
Member

Choose a reason for hiding this comment

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

Do we still also need to pass through a parent so that it uses the right provider instance? Or does this invoke not actually care about the provider? Even if it doesn’t now, should we pass the parent through regardless?

I actually don’t know for sure - so maybe input from @pgavlin would be good.

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 tried passing both args, and it was failing the same way (invoke returning an empty object {}). That may be a bug with the engine, but I don't know enough about it to be sure.

I think using the version makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Even if it doesn’t now, should we pass the parent through regardless?

I think the version makes the most sense here as well. This invoke does not need the particular provider instance (yet, at least).

@@ -121,16 +121,11 @@ import * as outputs from "../types/output";
resourcePrefix?: string;
}

function yamlLoadAll(text: string, opts?: pulumi.CustomResourceOptions): Promise<any[]> {
function yamlLoadAll(text: string): Promise<any[]> {
// Rather than using the default provider for the following invoke call, determine the
// provider from the parent if specified, or fallback to using the version specified
// in package.json.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment block seems outdated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, good catch.

@EronWright
Copy link
Contributor

Hopefully it will be still be possible to use k8s.yaml.ConfigGroup and k8s.yaml.ConfigFile in conjunction with a provider. An obvious scenario is creating a K8s cluster and then applying a manifest to it, in a single program.

@lblackstone
Copy link
Member Author

@EronWright Right, this change won't affect that functionality. This change is an implementation detail that shouldn't be visible to users.

@lblackstone lblackstone merged commit de362a8 into master Feb 11, 2020
@pulumi-bot pulumi-bot deleted the lblackstone/invoke-version branch February 11, 2020 22:20
@EronWright
Copy link
Contributor

@lblackstone I'm trying to use 1.5.3 and seeing errant behavior, that the ConfigFile is applied to the ambiant kube provider, not the passed-in provider.

@lblackstone
Copy link
Member Author

@EronWright Can you open an issue with details? I just double-checked locally, and wasn't able to reproduce. I created a new GKE cluster + ConfigFile and it worked as expected.

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

5 participants