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

Add RawInput to workaround double-serialization issue #966

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

jviau
Copy link
Collaborator

@jviau jviau commented Sep 13, 2023

This PR is to address a conflicting need in Durable Functions extension. We want to use TaskHubClient from within LocalGrpcListener so that we leverage all the logic there, especially the distributed tracing work. However, our input is already serialized and TaskHubClient would double serialize it. To work around this an "internal" API called RawInput has been added. When this type is used as the input for TaskHubClient, we will directly use RawInput.Value instead of serializing the input.

RawInput is "internal" in the sense that it is public, but in an .Internal namespace, with an [Obsolete] attribute, and xmldoc advising customers to use the type at their own risk. This is a pattern used in a few .NET libraries (efcore, webjobs, functions worker, etc) to address the internal-yet-public functionality.

Copy link
Collaborator

@davidmrdavid davidmrdavid left a comment

Choose a reason for hiding this comment

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

This makes sense to me. Tagging @cgillum for a final approval

Copy link
Collaborator

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

LGTM - I assume part of the reason why we're marking this [Obsolete] is because it's not really a fully fleshed-out feature (can't use RawValue in any other contexts currently)?

@jviau
Copy link
Collaborator Author

jviau commented Sep 19, 2023

I assume part of the reason why we're marking this [Obsolete] is because it's not really a fully fleshed-out feature (can't use RawValue in any other contexts currently)?

Essentially - it is used to raise a flag to customers that while this type is public, we don't want them to use it. So, every usage of it comes with a pseudo "I agree" via suppressing the warning.

@jviau jviau merged commit d7c3eb4 into Azure:main Sep 19, 2023
2 checks passed
@jviau jviau deleted the raw-input branch September 19, 2023 17:02
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.

None yet

3 participants