Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.17k stars 4.53k forks source link

Add clientBuilder extensions to data plane SDKs #35111

Open pallavit opened 1 year ago

pallavit commented 1 year ago

Our design guidelines say we need to implement the following - https://azure.github.io/azure-sdk/dotnet_implementation.html#integration-with-aspnet-core. However some of the data plane SDKs do not follow this pattern. Creating a follow-up bug to evaluate these for the following SDK

lirenhe commented 1 year ago

@ArcturusZhang, please help to check why the extension is not generated for Azure.Analytics.Purview.Administration as it is also released by using DPG.

ArcturusZhang commented 1 year ago

@pallavit We only implemented asp.net extension generation in DPG. The only package from DPG is Azure.Analytics.Purview.Administration, and its asp.net extension is explicitly suppressed by this file last time I introduced this feature, because in this package, we are introducing multiple top level clients with different client options. We did not support to generate this yet in the generator therefore generator cannot generate a proper version of asp.net for those clients that have manually written client options. I also cannot reenable this feature for this pacakge because those clients are customized too much to let the generator recognize them therefore it is currently impossible to generate extensions that could compile. Per offline discussions, I will manually add an extension to this library to support this in this library

pallavit commented 1 year ago

Thanks for letting me know. Does the generator support this scenario today - i.e. generate builders when multiple top-level clients are available or is this expected to be manual for those scenarios?

ArcturusZhang commented 1 year ago

Hi @pallavit currently in the generator, we have explicitly blocked the combination of configuration generation1-convenience-client: true and public-clients: true (details in this issue) Therefore HLC packages will never have a generated public client, and this makes the generator impossible to generate client builder for them - because from the generator's perspective, it never knows a public clients in those packages.

Therefore, given the context above, it is reasonable to say, it is by-design that you have to write the client builder extensions along with your customization code of your new public clients in HLC packages. And I think the corresponding owners of those HLC packages should follow up on this issue to get the package updated with the client builder extensions.

pallavit commented 1 year ago

Sounds good. Thank you for clarification here.