[sdk-gen] Use specialized missing required property exception #1228
Conversation
| if prop.IsRequired() && prop.DefaultValue == nil && prop.ConstValue == nil { | ||
| fprintf(w, " if ($.%s == null) {\n", fieldName) | ||
| fprintf(w, " throw new %s(\"%s\", \"%s\");\n", | ||
| ctx.ref(names.PulumiMissingRequiredPropertyException), pt.name, fieldName) |
There was a problem hiding this comment.
Are we going to need to first ship the exception in the core Java SDK, and then somehow have the codegen indicate that version of the core Java SDK is now required (since it depends on this new exception being there)? I'm not familiar enough with Java on how to indicate that requirement.
There was a problem hiding this comment.
I thought it should be fine because if it is merged and tag a release, a new sdk is published that includes the new exception type, sometime later providers team will pick up the newer sdk-gen from pulumi-java and start using that new type. This is assuming that sdk-gen references latest pulumi-java sdk
There was a problem hiding this comment.
sdk-gen dictates the version of the core library to use. So we need to release the type first, then make sure sdk-gen is setting that version as the required version when emiting the provider sdks.
There was a problem hiding this comment.
i.e. "This is assuming that sdk-gen references latest pulumi-java sdk" is the assumption that doesn't hold. e.g. this line which will be set from in https://github.com/pulumi/pulumi-java/blob/main/pkg/codegen/java/templates_gradle.go#L87 somehow.
There was a problem hiding this comment.
i.e. "This is assuming that sdk-gen references latest pulumi-java sdk" is the assumption that doesn't hold.
The pulumi-java-gen CLI used to generate java SDKs takes the version of the pulumi-java main sdk as input but it defaults to the version of the pulumi-java-gen itself, see here:
cmd.Flags().StringVar(&javaSdkVersionArg, "sdk", version.Version,
"com.pulumi:pulumi version to depend on in the generated code")Doesn't that mean if we tag 0.9.x which publishes both SDK and generator, that the generated SDKs will be using 0.9.x as well? 🤔
There was a problem hiding this comment.
What about from the Pulumi cli where we call the gen methods directly?
There was a problem hiding this comment.
Tested what Pulumi CLI does when you run:
pulumi package gen-sdk schema.json --language java --out sdk
and it generates just java source files without a gradle project file 😱 so basically gen-sdk isn't usable with java projects
There was a problem hiding this comment.
🤦♂️ alright, well we'll need to look into that at some point but means nothing to worry about for this PR
# Description So that it can be used from #1228 to fix #1199 ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [ ] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have updated the [CHANGELOG-PENDING](https://github.com/pulumi/pulumi/blob/master/CHANGELOG_PENDING.md) file with my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- @pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
Zaid-Ajaj
force-pushed
the
use-specialized-missing-required-property-exception
branch
from
7932305 to
8196203
Compare
Zaid-Ajaj
deleted the
use-specialized-missing-required-property-exception
branch
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
for Pulumi's contribution guidelines.
Help us merge your changes more quickly by adding more details such
as labels, milestones, and reviewers.-->
### Proposed changes
This PR updates the pulumi-java dependency from v0.8.0 to v0.12.0.
Notable changes:
- pulumi/pulumi-java#1356
- pulumi/pulumi-java#1361
- pulumi/pulumi-java#1228
### Related issues (optional)
<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->
Description
This PR adds a new custom exception
MissingRequiredPropertyExceptionto the main Java SDK. Then SDK-gen uses this type and throws it as an error when an input property is required but isn't specified. This fixes #1199 where previously the users would receive aNullPointerExceptioninstead.Due to using Go templates and conditionals as follows:
The generated code emits an empty line when
IsRequiredisfalsewhich is why you will see the new line diffs in the commits of this PRChecklist