Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.06k stars 1.19k forks source link

[Deprecate]: deprecate apiVersion in ClientOptions from core-client-rest #29883

Open qiaozha opened 3 months ago

qiaozha commented 3 months ago

Package Name

@azure-rest/core-client

Deprecated Versions

none

Deprecation Message

not sure yet?

Why is deprecation necessary?

It's not about deprecating the whole package. Currently, we put apiVersion in the ClientOptions in core client rest https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/core/core-client-rest/src/common.ts#L331 and have a apiVersionPolicy to append the apiVersion query parameter that customer passes in the clientOptions https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/core/core-client-rest/src/apiVersionPolicy.ts#L14, however, not every operation inside a client has apiVersion parameter and even if it has, it may not be a query parameter, this is especially true for non Azure services, currently, we have add https://github.com/Azure/autorest.typescript/blob/81e09d148e33ce8d24e1746a7913782e621b8f6c/packages/typespec-ts/test/integration/generated/type/scalar/src/scalarClient.ts#L34 remove this policy logic and log a warning here to reduce customers confusion.

But we should actually get rid of apiVersion here as discussed here https://github.com/Azure/autorest.typescript/issues/2427#issuecomment-2068360031 and generate it in the client level and add the policy here if this client really does support client level apiVersion.

qiaozha commented 3 months ago

After rethinking, if we want to completely get rid of the apiVersion and apiVersionPolicy in the core-client-rest. we probably could take the following steps to minimize the impact.

  1. generate the apiVersion and another apiVersionPolicy with a different name policy in the generated client and add that policy when it applies. in this way, the current apiVersionPolicy actually changes nothing even it applies.
  2. change the removePolicy code here to if the current apiVersionPolicy exists then remove it.
  3. refresh all the RLC clients that are impacted with a minor version upgrade.
  4. mark the apiVersion in ClientOptions as deprecated from core-client-rest.
  5. after sometime, we can remove the apiVersion in ClientOptions and remove the apiVersionPolicy then bump the major version in core-client-rest and the dependency in generated code.

@xirzec @joheredi how do you think?

xirzec commented 3 months ago

@qiaozha this sounds like a good plan to me.