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

Add Output.JsonSerialize to dotnet sdk #11556

Merged
merged 1 commit into from Dec 9, 2022
Merged

Conversation

Frassle
Copy link
Member

@Frassle Frassle commented Dec 6, 2022

Description

Plan is to add functions like this to all the SDKs. JsonSerialization is very language specific, dotnet for example uses System.Text.Json, go would use JsonMarshal, etc. So it's worth having it built into SDKs and then exposed as a PCL intrinsic (with the caveat that the cross-language result will be valid JSON, but with no commmitment to formatting for example).

This is just the first part of this work, to add it to the dotnet SDK (simply because I know that best).

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Dec 6, 2022

Changelog

[uncommitted] (2022-12-09)

Features

  • [sdk/dotnet] Add Output.JsonSerialize using System.Text.Json.
    #11556

@Frassle
Copy link
Member Author

Frassle commented Dec 6, 2022

N.B. Luke looked at a similar function for NodeJS last hackathon: https://github.com/pulumi/pulumi/compare/lukehoban/outputhelpers

I'm not sure we want to add all those functions in that branch as intrinsics but I'll commit to getting the object -> json string method done.

@Frassle
Copy link
Member Author

Frassle commented Dec 6, 2022

@Zaid-Ajaj made the great point that this should probably handle nested outputs in the input structure. I'll see if we can support that properly as part of this.

@aq17 aq17 force-pushed the fraser/dotnetJsonSerialize branch from 0ff1c20 to a0b9c83 Compare December 6, 2022 19:01
@Frassle
Copy link
Member Author

Frassle commented Dec 7, 2022

@AaronFriel's asked this to hold for a bit while we work out if access to the languages JSON serialization is the right approach, or if we should add this to pulumi-std so it always writes the JSON in exactly the same way and then provide a way for users to hook how pulumi marshals/unmarshals from pulumi property values to language types.

@Frassle Frassle mentioned this pull request Dec 8, 2022
3 tasks
@Frassle Frassle force-pushed the fraser/dotnetJsonSerialize branch 4 times, most recently from f7d4bfb to 45ea5af Compare December 8, 2022 21:59
@Frassle
Copy link
Member Author

Frassle commented Dec 8, 2022

This doesn't update codegen, we should update codegen but it looks to be simpler to do a release with this in and then update codegen to use it in the next release.

@AaronFriel
Copy link
Member

For posterity and in case I find this PR in the future, I've retracted any issue I had with this in favor of doing the least surprising thing in each language, and to push the complexity into YAML and pulumi convert if we choose to try to preserve semantics across conversion.

Copy link
Contributor

@Zaid-Ajaj Zaid-Ajaj left a comment

Choose a reason for hiding this comment

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

Looks great, very nice addition to the SDK

Plan is to add functions like this to _all_ the SDKs.
JsonSerialization is _very_ language specific, dotnet for example uses
System.Text.Json, go would use JsonMarshal, etc. So it's worth having it
built into SDKs and then exposed as a PCL intrinsic (with the caveat
that the cross-language result will be _valid_ JSON, but with no
commmitment to formatting for example).

This is just the first part of this work, to add it to the dotnet SDK
(simply because I know that best).
@Frassle
Copy link
Member Author

Frassle commented Dec 9, 2022

bors merge

@bors
Copy link
Contributor

bors bot commented Dec 9, 2022

Build succeeded:

@bors bors bot merged commit a0f2c1c into master Dec 9, 2022
@bors bors bot deleted the fraser/dotnetJsonSerialize branch December 9, 2022 12:51
abhinav pushed a commit to pulumi/pulumi-dotnet that referenced this pull request Jan 11, 2023
11556: Add Output.JsonSerialize to dotnet sdk r=Frassle a=Frassle

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Plan is to add functions like this to _all_ the SDKs. JsonSerialization is _very_ language specific, dotnet for example uses System.Text.Json, go would use JsonMarshal, etc. So it's worth having it built into SDKs and then exposed as a PCL intrinsic (with the caveat that the cross-language result will be _valid_ JSON, but with no commmitment to formatting for example).

This is just the first part of this work, to add it to the dotnet SDK (simply because I know that best).


## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [x] 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 run `make changelog` and committed the `changelog/pending/<file>` documenting 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. -->


Co-authored-by: Fraser Waters <fraser@pulumi.com>
@justinvp justinvp mentioned this pull request Jan 21, 2023
31 tasks
@lukehoban lukehoban mentioned this pull request Mar 27, 2023
19 tasks
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

4 participants