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

[Azure.AI.Translation.Document] .NET 6+ preprocessor directives removed due to code not compiling #47131

Open
m-redding opened this issue Nov 12, 2024 · 1 comment
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Translator Service Attention Workflow: This issue is responsible by Azure service team.

Comments

@m-redding
Copy link
Member

Previously Azure.AI.Translation.Document only included a netstandard2.0 target. In DocumentTranslateContent.Serialization.cs there were two lines of code inside preprocessor directives for .NET 6+. Since there was not a net6.0+ target, these lines of code were not being compiled. When a net8.0 target was added, these lines of code failed to compile because the code was out of date from when it was written. In order to preserve existing behavior, they were removed.

This is not a regression because they were not being used previously.

The library author(s) should consider whether these should be added back and how to fix the code if so.

See L60-67:

#if NET6_0_OR_GREATER
				writer.WriteRawValue(global::System.BinaryData.FromStream(item));
#else
                    using (JsonDocument document = JsonDocument.Parse(BinaryData.FromStream(item.Content)))
                    {
                        JsonSerializer.Serialize(writer, document.RootElement);
                    }
#endif

See L41-48:

#if NET6_0_OR_GREATER
				writer.WriteRawValue(global::System.BinaryData.FromStream(Document));
#else
            using (JsonDocument document = JsonDocument.Parse(BinaryData.FromStream(MultipartDocument.Content)))
            {
                JsonSerializer.Serialize(writer, document.RootElement);
            }
#endif

See: https://github.com/Azure/azure-sdk-for-net/pull/46637/files#r1819709439

@github-actions github-actions bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Nov 12, 2024
@m-redding m-redding added the Client This issue points to a problem in the data-plane of the library. label Nov 12, 2024
@github-actions github-actions bot removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Nov 12, 2024
@m-redding m-redding changed the title [Azure.AI.Translation.Document] Potential performance improvement [Azure.AI.Translation.Document] .NET 6+ preprocessor directives removed due to code not compiling Nov 12, 2024
@m-redding m-redding added the Service Attention Workflow: This issue is responsible by Azure service team. label Nov 12, 2024
Copy link

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @MikeyMCZ @swmachan @vikaspalaskar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Translator Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

No branches or pull requests

1 participant