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 invokes with non-object outputs, via wrapping #3165

Merged
merged 6 commits into from
Apr 10, 2024

Conversation

thomas11
Copy link
Contributor

@thomas11 thomas11 commented Mar 21, 2024

  • Add invokes that return non-object types such as plain strings
  • AzureClient.Get and .Post can deserialize any response, not just objects
  • regenerate

Resolves #3147

If we're generating response properties and the resolved schema has no properties but is a primitive/array output, we include it as a special property so invokes can be generated.

Ideally we'd just represent the response as a primitive type without the wrapping magic property, using FunctionSpec.ReturnType, but pulumi/pulumi#15738 and pulumi/pulumi#15739 prevent this.

Unfortunately, this change required adding a third boolean to genProperties to limit this new case to function responses only. Happy to incorporate suggestions to model this better.

This is a simple program that demonstrates one of the new invokes.

import * as app from "@pulumi/azure-native/app";
const id = app.getCustomDomainVerificationIdOutput();
export const customDomainVerificationId = id;
export const customDomainVerificationIdValue = id.value;

Output:

  + customDomainVerificationId     : {
      + value: "B11..."
    }
  + customDomainVerificationIdValue: "B11..."

Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New functions:

  • app.getCustomDomainVerificationId
  • dbforpostgresql.getGetPrivateDnsZoneSuffixExecute
  • network.getVirtualNetworkGatewayConnectionIkeSas
  • network.getVirtualNetworkGatewayVpnProfilePackageUrl
  • network.getVpnLinkConnectionIkeSas

Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 85.54217% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 55.25%. Comparing base (065e8dc) to head (3389689).

Files Patch % Lines
provider/pkg/provider/crud/crud.go 50.00% 6 Missing and 1 partial ⚠️
...ustomresources/custom_blob_container_legal_hold.go 50.00% 2 Missing and 1 partial ⚠️
provider/pkg/gen/properties.go 96.77% 0 Missing and 1 partial ⚠️
provider/pkg/provider/provider.go 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3165      +/-   ##
==========================================
+ Coverage   55.04%   55.25%   +0.21%     
==========================================
  Files          66       66              
  Lines        9857     9908      +51     
==========================================
+ Hits         5426     5475      +49     
+ Misses       4002     4001       -1     
- Partials      429      432       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@thomas11 thomas11 changed the title Support invokes with non-object outputs Support invokes with non-object outputs, via wrapping Mar 21, 2024
Copy link
Member

@danielrbradley danielrbradley left a comment

Choose a reason for hiding this comment

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

Looks good overall - the reasoning looks solid and nicely factored commits 👍

One optional consideration around the growing list of magic bools

If the schema has no properties but is a primitive output, we include it as a property so invokes can be generated.
Ideally we'd just represent it as a primitive type and use FunctionSpec.ReturnType to specify this, but pulumi/pulumi
15739 and 15738 prevent this.

Unfortunately, this required adding an unfortunate third boolean param
to genProperties to limit this new case to function responses only.
Since commit 308846d requires the non-object responses to be wrapped
in a certain object, ResourceCrudClient does that.
@thomas11 thomas11 force-pushed the tkappler/non-object-invokes branch from fb1a6e2 to 5c49672 Compare April 9, 2024 13:30
func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput, isType, isResponse bool) (*propertyBag, error) {
// genPropertiesVariant is a set of flags that control the behavior of property generation
// in genProperties and genProperty
type genPropertiesVariant struct {
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍

@thomas11 thomas11 merged commit 3ce1712 into master Apr 10, 2024
23 checks passed
@thomas11 thomas11 deleted the tkappler/non-object-invokes branch April 10, 2024 04:10
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.

Add a function for the getCustomDomainVerificationId API.
2 participants