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.

Sign up for GitHub

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

Jump to bottom

Enable JSON Patch in AOT #1588

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enable JSON Patch in AOT #1588

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/KubernetesClient.Aot/KubernetesJson.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ public static TValue Deserialize<TValue>(Stream json, JsonSerializerOptions json

public static string Serialize(object value, JsonSerializerOptions jsonSerializerOptions = null)
{
if (value is V1Patch { Content: string jsonValue })
Copy link
Member

Choose a reason for hiding this comment

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

i did not get it, is it still some dynamic thing?

Copy link
Author

Choose a reason for hiding this comment

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

I don't quite understand the question?

In the usages I've seen, the Content property of the V1Patch object is always set as the json string value of the patch request body. This pattern, broken down, checks:

value is of type V1Patch, and is not null
value.Content is of type string
Then it assigns the value of value.Content to jsonValue, however this is all essentially just syntactic sugar, so all of that is resolved at compile time into a slightly uglier string of conditions.

Copy link
Member

Choose a reason for hiding this comment

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

let me ask this way?
is it a breaking change or different behavior from non-aot version?
the assumption is that V1Patch must be json string?

Copy link
Author

@hwoodiwiss hwoodiwiss Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ahhh, got you, this isn't a breaking behaviour, and I would say it's restoring some functionality that exists in the non-aot version to the aot version.

This only restores the ability to use json string in the json patch, however I don't think full parity can be achieved without changing the API surface.

{
return jsonValue;
}

var info = SourceGenerationContext.Default.GetTypeInfo(value.GetType());
return JsonSerializer.Serialize(value, info);
}
Expand Down
Loading