Skip to content

Support optional client-level parameter derived from x-ms-parameterized-host #1635

@MaryGao

Description

@MaryGao

Background

In recent RLC release we notice the requirement to handle optional client-level parameters in codegen.

In our current implementation we put these parameters in the signature of client factory. This could work fine for required parameter without default value. However if there exists default value we couldn't leverage it defined in swagger.

Current implementation

If we set the x-ms-parameterized-host as

  "x-ms-parameterized-host": {
    "hostTemplate": "{Endpoint}/atlas/{serviceVersion}",
    "useSchemePrefix": false,
    "parameters": [
      {
        "$ref": "#/parameters/Endpoint"
      },
      {
        "$ref": "#/parameters/serviceVersion"
      }
    ]
  },

and generated client constructor factory function look like this

export default function createClient(
  Endpoint: string,
  credentials: TokenCredential,
  serviceVersion: string,
  options: ClientOptions = {}
): PurviewCatalogClient {
  const baseUrl =
    options.baseUrl ?? `${Endpoint}/atlas/{serviceVersion}/catalog/api`;
    // ...
}

Proposal

Option 1(preferred) - Put necessary parameter into XXXClientOption interface

export interface AzureDevCenterClientOption extends ClientOptions {
   devCenterDnsSuffix: string;
}

export default function createClient(
  tenantId: string,
  devCenter: string,
  credentials: TokenCredential,
  options: AzureDevCenterClientOption = {}
): AzureDevCenterClient { 
  // Handle the default value here
}

Option 2 - Put the default value in method signature

export default function createClient(
  tenantId: string,
  devCenter: string = "asia",  credentials: TokenCredential,
  devCenterDnsSuffix: string = "devcenter.azure.com",  options: ClientOptions = {}
): AzureDevCenterClient {
  const baseUrl = options.baseUrl ?? `https://${tenantId}-${devCenter}.${devCenterDnsSuffix}`

We have two concerns

  1. Position: Usually we put the default value at the last position of the method. But if the parameter from required -> optional, it could involve the position change, finally it could introduce breaking change.
  2. Another concern is for callee to use the method. Like the below definition if the user wants to use the default value, with devCenter and devCenterDnsSuffix but inputs the configured options, he needs to call this way:
const client = createClient("tenantId", undefined, credentials, undefined, { customized options });

PS: To support apiVersion parameter we have dependency in autorest bug Azure/autorest#4656.

/cc @qiaozha @joheredi

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions