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

fix: Default JSON serializer instance is not affected by any global settings. #2613

Merged
merged 1 commit into from Nov 17, 2023

Conversation

amanda-tarafa
Copy link
Contributor

This partially superceeds #2610 and it's a better approach overall as our serializar is not created using the global settings at all, so we don't need to override global settings. Note that JsonConvert.Deserialize uses JsonSerializer.CreateDefault that relies on global settings, and then applies any specific settings. In turn we use JsonSerializer.Create that only applies specific settings. By not using JsonConvert we have stopped being affected by global settings at all. The test from #2610 remains and proves this approach works.

Possibly fixes #2611

…ettings.

This partially superceeds #2610 and it's a better approach overall as our serializar is not created using the global settings at all, so we don't need to override global settings.
Note that JsonConvert.Deserialize uses JsonSerializer.CreateDefault that relies on global settings, and then applies any specific settings. In turn we use JsonSerializer.Create that only applies specific settings. By not using JsonConvert we have stopped being affected by global settings at all.
The test from #2610 remains and proves this approach works.

Possibly fixes #2611
Copy link
Collaborator

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Thanks for figuring out how to do this better :)

@amanda-tarafa amanda-tarafa merged commit a7ba66c into main Nov 17, 2023
6 checks passed
@amanda-tarafa amanda-tarafa deleted the json-global-settings branch November 17, 2023 06:44
amanda-tarafa added a commit that referenced this pull request Jan 9, 2024
Fixes:

- #2613 Default JSON serializer is not affected by gloabl JSON settings.
- #2630 Tune authorization token staleness and validity windows so that it doesn't overlap with Metadata Server authorization token caching.

Features:

- #2616 Makes batch response handling more efficient, particularly benefiting large responses.
amanda-tarafa added a commit that referenced this pull request Jan 9, 2024
Fixes:

- #2613 Default JSON serializer is not affected by gloabl JSON settings.
- #2630 Tune authorization token staleness and validity windows so that it doesn't overlap with Metadata Server authorization token caching.

Features:

- #2616 Makes batch response handling more efficient, particularly benefiting large responses.
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.

[Error deserializing JSON credential data]
2 participants