Expicitly block public clients + convenience

[m-nash]

We are currently allowing an unsupported combination of flags in autorest when public-clients and generation1-convenience-client are both true.

This combination is especially problematic when it comes to LRO since it exposes publicly the Start prefixed method names which we don't do any longer and doesn't use the WaitUntil parameter.

Unfortunately we have 5 libraries that currently use this problematic combination. 4 are beta

• [ ] https://www.nuget.org/packages/Azure.Analytics.Synapse.Spark
• [ ] https://www.nuget.org/packages/Azure.Analytics.Synapse.Monitoring
• [ ] https://www.nuget.org/packages/Azure.Analytics.Synapse.ManagedPrivateEndpoints
• [ ] https://www.nuget.org/packages/Azure.Analytics.Synapse.Artifacts

one is GA

• [ ] https://www.nuget.org/packages/Azure.Communication.PhoneNumbers

We need to error in the generator if both of these flags are true

To support backwards compatibility we will port the generated portion that exists to custom code in order to maintain the existing api shape.

[DominikMe]

@m-nash For PhoneNumbers we had discussed options with Azure Eng and this was the only solution available for generating LROs for autorest + convenience track 2 data plane libraries. Before making this an error, what is the supported alternative?

[DominikMe]

As a service family we're not ready to adopt DPG at this point.

[m-nash]

The supported answer is to write the convenience layer by hand as a wrapper to the internal clients similar to what is done for other languages. Since PhoneNumbers is already GA with this setting we will grandfather that single project in to not disrupt what is there.

That being said the only reason Email ran into a blocker and PhoneNumbers did not was because they had LROs and the generated pattern for LRO is not meant for public consumption. If PhoneNumbers ever introduces an LRO they will hit the same API shape issue and at that time will need to convert to a hand written wrapper.

[DominikMe]

@m-nash PhoneNumbers has true LROs in the service, it's not about introducing them. The Start* methods are part of the convenience layer client and back then followed the naming guideline for .NET.

I think there's one misunderstanding here: Even though PhoneNumbers set public-clients it's making the generated client internal and doesn't expose it, see https://github.com/Azure/azure-sdk-for-net/blob/403647170755aa26ca3f55891c17ef3ed9787af6/sdk/communication/Azure.Communication.PhoneNumbers/src/InternalPhoneNumbersClient.cs#L9

[DominikMe]

Email can definitely look into directly extending Operation<T> and completely hand-roll in their convenience layer. We'll get back here if issues come up.

[apattath]

JFYI - For Email, I'm following the Azure Blob Storage's pattern from here to hand-roll the Operation inherited class: https://github.com/apattath/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Blobs/src/Models/CopyFromUriOperation.cs#L19

[zaowang-ms]

Hi there, thanks for mentioning the error. While I'm not very sure of why this change is needed, one of our internal projects is influenced by the change. We are unable to generate our client with both flags are true. I'm wondering is this just a temporary error or we may need to pay attention and change our configuration in the future?

[m-nash]

@zaowang-ms The intent here was to enforce our standards on the azure sdk it was unintended to block external libraries who might have different standards but still use autorest.csharp. I have opened this issue to track that separately.