-
Notifications
You must be signed in to change notification settings - Fork 21
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
Implement GetRequiredPackages
for Java
#1535
Conversation
c6c1fe5
to
3459c61
Compare
ctx context.Context, | ||
req *pulumirpc.GetRequiredPluginsRequest, | ||
) (*pulumirpc.GetRequiredPluginsResponse, error) { | ||
logging.V(5).Infof("GetRequiredPlugins: program=%v", req.GetProgram()) //nolint:staticcheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was removing this log intentional? It still seems useful to know when GetRequirePackages is called (though we are pretty inconsistent with logging overall)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was -- I checked the PR in pulumi/pulumi that added GetRequiredPackages
for Go, NodeJS and Python, and it seemed like we weren't doing any logging there really for those calls, so I figured I'd remove it. But I can add something back before I merge -- agree it doesn't hurt 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah fair enough. I didn't double check for those. Might be worth adding them there for consistency ;)
context.Context, | ||
*pulumirpc.GetRequiredPluginsRequest, | ||
) (*pulumirpc.GetRequiredPluginsResponse, error) { | ||
return nil, status.Errorf(codes.Unimplemented, "method GetRequiredPlugins not implemented") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if we still need to implement this, so it works with an older engine. But language runtime plugins are always bundled, so we know we'll always call GetRequiredPackages first anyway. Good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On both your comments, I've tried to match what was done for Go, NodeJS and Python -- it seems like we did the add/remove simultaneously for them and as you say, this is bundled so I think it's OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this is fine, I just stumbled over it, so wanted to double check my thought process :)
baddcfd
to
e8fc393
Compare
As part of a Pulumi deployment, the Pulumi engine will ask the language host running the program for the set of plugins required to run that program successfully by making a `GetRequiredPlugins` gRPC call. For instance, if one has a TypeScript program whose `index.ts` `import`s `@pulumi/aws`, the NodeJS language host will find the `@pulumi/aws` package and report that the `aws` resource plugin is required. This enables features such as pre-emptively downloading a plugin ahead of time, as opposed to waiting for the first resource registration against that plugin. With the introduction of parameterized providers, `GetRequiredPlugins` falls a little short -- parameterization means that it is no longer necessarily the case that a package named `x` is provided by a plugin of the same name. For instance, with a dynamically bridged Terraform provider X, the provider name will be X, but the plugin name will be e.g. `pulumi-terraform-provider`. In pulumi/pulumi#17894, `GetRequiredPackages` was added to the language host interface to provide the additional context of a plugin's parameterization, and is now used if present instead of `GetRequiredPlugins`. This change implements `GetRequiredPackages` for Java as follows: * Presently, `GetRequiredPlugins` is implemented by running a small Java program (located in the internal `bootstrap` package of the core SDK) that walks its class path looking for `pulumi-plugin.json` files. This code is extended to parse `parameterization` blocks from those JSON files, as we do in other languages using this mechanism. * When we generate SDKs, we generate Gradle build files which generate `pulumi-plugin.json` files as part of the build process. We extend the `build.gradle` template used when generating SDK build files so that it emits `parameterization` blocks appropriately. As part of this, we also clean up the emission of package names and versions. * We now have enough to run and pass the `l2-parameterized-resource` conformance test, which in theory confirms that all of this works, so we include that as part of this work. * We remove the now-deprecated `GetRequiredPlugins` implementation. Fixes #1510
e8fc393
to
e94090c
Compare
This conformance test can be enabled with the implementation of `GetRequiredPackages` from #1535.
This conformance test can be enabled now with the implementation of `GetRequiredPackages` from #1535.
As part of a Pulumi deployment, the Pulumi engine will ask the language host running the program for the set of plugins required to run that program successfully by making a
GetRequiredPlugins
gRPC call. For instance, if one has a TypeScript program whoseindex.ts
import
s@pulumi/aws
, the NodeJS language host will find the@pulumi/aws
package and report that theaws
resource plugin is required. This enables features such as pre-emptively downloading a plugin ahead of time, as opposed to waiting for the first resource registration against that plugin.With the introduction of parameterized providers,
GetRequiredPlugins
falls a little short -- parameterization means that it is no longer necessarily the case that a package namedx
is provided by a plugin of the same name. For instance, with a dynamically bridged Terraform provider X, the provider name will be X, but the plugin name will be e.g.pulumi-terraform-provider
. In pulumi/pulumi#17894,GetRequiredPackages
was added to the language host interface to provide the additional context of a plugin's parameterization, and is now used if present instead ofGetRequiredPlugins
. This change implementsGetRequiredPackages
for Java as follows:GetRequiredPlugins
is implemented by running a small Java program (located in the internalbootstrap
package of the core SDK) that walks its class path looking forpulumi-plugin.json
files. This code is extended to parseparameterization
blocks from those JSON files, as we do in other languages using this mechanism.pulumi-plugin.json
files as part of the build process. We extend thebuild.gradle
template used when generating SDK build files so that it emitsparameterization
blocks appropriately. As part of this, we also clean up the emission of package names and versions.l2-parameterized-resource
conformance test, which in theory confirms that all of this works, so we include that as part of this work.GetRequiredPlugins
implementation.Fixes #1510