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

Allow customization of JsonSerializationOptions #370

Open
rosieks opened this issue Oct 30, 2024 · 4 comments
Open

Allow customization of JsonSerializationOptions #370

rosieks opened this issue Oct 30, 2024 · 4 comments
Labels
kind/enhancement Improvements or new features

Comments

@rosieks
Copy link

rosieks commented Oct 30, 2024

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Right now to deserialize configuration Pulumi use default settings for JsonSerializer. That means that if someone wants to use to use different configuration then it's forced to use eg JsonPropertyName and others tools on each class which makes code harder to read. It would be great if there was an option to override it and provide custom JsonSerializerOptions.

Affected area/feature

@rosieks rosieks added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Oct 30, 2024
@justinvp justinvp removed the needs-triage Needs attention from the triage team label Nov 1, 2024
@justinvp
Copy link
Member

justinvp commented Nov 1, 2024

Make sense, thanks! There's already precedent for exposing System.Text.Json.JsonSerializerOptions from an API in our SDK:

public static Output<T> JsonDeserialize<T>(Output<string> json, System.Text.Json.JsonSerializerOptions? options = null)

For now, another alternative workaround would be to get the config as a string value and the deserialize the JSON yourself using whatever deserializer/options you'd like.

@rosieks
Copy link
Author

rosieks commented Nov 1, 2024

Easy to implement that is just add new constructor that allows you to provide options. If that's acceptable way I can provide PR.

@justinvp
Copy link
Member

justinvp commented Nov 1, 2024

I don't think it'd be a new constructor. I think it'd be new overloads on the Get*Object/Require*Object methods:

public T GetObject<T>(string key); // Existing
public T GetObject<T>(string key, System.Text.Json.JsonSerializerOptions options); // New

public Output<T>? GetSecretObject<T>(string key); // Existing
public Output<T>? GetSecretObject<T>(string key, System.Text.Json.JsonSerializerOptions options); // New

public T RequireObject<T>(string key); // Existing
public T RequireObject<T>(string key, System.Text.Json.JsonSerializerOptions options); // New

public Output<T> RequireSecretObject<T>(string key); // Existing
public Output<T> RequireSecretObject<T>(string key, System.Text.Json.JsonSerializerOptions options); // New

I can provide PR

That'd be great, thanks! But let me discuss the proposal with the team first in our design meeting early next week.

@justinvp
Copy link
Member

justinvp commented Nov 8, 2024

@rosieks, if you're still interested, we discussed the APIs I outlined above and are good add those, if you'd like to provide a PR. Please include tests. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

2 participants