-
Notifications
You must be signed in to change notification settings - Fork 18
Create .NET RPC sample that doesn't use codegen #1078
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
base: main
Are you sure you want to change the base?
Conversation
| { | ||
| } | ||
|
|
||
| public class Utf8JsonSerializer : IPayloadSerializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please just add the Utf8JsonSerializer into the SDK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not. This library's core is not about serialization. If we pin ourselves to any particular serialization package (even System.Text.Json) we open ourselves up to inevitable security issues from that library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've already gone through security reviews for this library where we were explicitly told that this is the correct way to handle serialization libraries. Changing course now may require another round of security reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the code generation, would create this class I don't see the point, in the end tooling from us would cause this dependency in the application of the customer. Also, System.Text.Json is part of dotnet framework and no separate nuget anymore and used in other places also.
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <ProjectReference Include="..\..\..\src\Azure.Iot.Operations.Mqtt\Azure.Iot.Operations.Mqtt.csproj" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use this as real customer example and use the nugets instead of the project reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that matters for this exercise, but sure, I can do that.
|
@timtay-microsoft is there already an example on how to use TelemetryReceiver without code gen in the repository? If not would you be so kind to add it? |
I'm not sure if we actually need this, but it sounds like it may be helpful to confirm any feedback we receive from @koepalex