search

microsoft / kiota-serialization-json-python

JSON serialization implementation for Kiota clients in Python
https://aka.ms/kiota/docs
MIT License
4 stars 12 forks source link

Adding dict[str, Any] support #329

Closed Acurisu closed 4 months ago

Acurisu commented 4 months ago

Overview

Several classes contain dict[str, Any] or even list[dict[str, Any]] inside their additional_data which is currently not handled.

Related Issue

~

Demo

~

Notes

As I'm unsure about certain design decision this might not be the intended way of solving this. Since I only tested this in the cases where it affects me it might not cover all cases.

Testing Instructions

Acurisu commented 4 months ago

@microsoft-github-policy-service agree

sonarcloud[bot] commented 4 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Acurisu commented 4 months ago

On the topic of making methods private so noone takes a dependency on it as they are unique for the JSON serializer. Shouldn't write_any_value also be private?

And I chose to make the methods "private" now using __ so they are really mangled, but would you prefer them being "semiprivate" like the other methods such as _serialize_value instead?

baywet commented 4 months ago

@Acurisu linting is failing, and I don't have permissions to push to your fork. Can you apply the following patch on your end please? 0001-chore-linting-fixes.patch

baywet commented 4 months ago

On the topic of making methods private so noone takes a dependency on it as they are unique for the JSON serializer. Shouldn't write_any_value also be private?

Yes I guess it should have been since it's not in the interface. However changing it now would be a source breaking change. SO let's leave it as is.

And I chose to make the methods "private" now using __ so they are really mangled, but would you prefer them being "semiprivate" like the other methods such as _serialize_value instead?

Private is fine, if we get feedback people need to access those methods for some reason, we can always relax things later.

Acurisu commented 4 months ago

@Acurisu linting is failing, and I don't have permissions to push to your fork. Can you apply the following patch on your end please?

0001-chore-linting-fixes.patch

Will do so. Seems like the linter config didn't get applied on my end, my bad.

baywet commented 4 months ago

actually hold on, I'm going to make the changes on my end since this is blocking a few things here. Sorry about the confusion.