Remove static cache #47859
Remove static cache #47859
Conversation
live1206
requested review from
JoshLove-msft,
m-nash,
jorgerangel-msft,
jsquire and
annelo-msft
as code owners
github-actions
bot
added
Client
| var restClientNamesToExclude = new HashSet<string>(); | ||
| foreach(var provider in baseProviders) | ||
| { | ||
| // skip clients without methods |
There was a problem hiding this comment.
Shouldn't we only do this for ARM libraries?
There was a problem hiding this comment.
I think we should override CreateClientCore in the AzureTypeFactory, and just return null for the case we care about. I'm adding support for returning null in microsoft/typespec#5638
There was a problem hiding this comment.
Shouldn't we only do this for ARM libraries?
As in this PR description, I am not sure if we should diverge this for MPG and DPG. Is there a good reason why this is only for MPG?
There was a problem hiding this comment.
I think we should override CreateClientCore in the AzureTypeFactory, and just return null for the case we care about. I'm adding support for returning null in microsoft/typespec#5638
OK, that's one way to deal with it, I will update the implementation.
There was a problem hiding this comment.
@JoshLove-msft Actually, it would be hard to check if the client is empty in CreateClientCore with only InputClient.
Because we also need to deal with sub-clients which is populated within clientProvider, if the client contains empty sub-clients, we should skip them. But if the sub-clients contains methods, we should not skip the root client.
https://github.com/Azure/azure-sdk-for-net/pull/47859/files#diff-64b049a6da4931ec4ae5b7a5d3fc110140166f14857b9e612167d4450c04a5e8R52-R79 is the corresponding test.
There was a problem hiding this comment.
Ok, in that case can we use a visitor to handle this?
There was a problem hiding this comment.
Ok, in that case can we use a visitor to handle this?
hmm, not easy to achieve this with visitor, either. How can we omit a provider using visitor?
There was a problem hiding this comment.
As offline discussed, will handle this in MGC as general behavior.
Follow-up work is in microsoft/typespec#5652
Remove static cache, it is causing unexpected cache state shared between isolated tests.