Skip to content

Remove apiVersion pipeline policy and move default apiVersion value to operation level#3702

Merged
JialinHuang803 merged 14 commits intoAzure:mainfrom
JialinHuang803:remove-apiver-policy
Jan 30, 2026
Merged

Remove apiVersion pipeline policy and move default apiVersion value to operation level#3702
JialinHuang803 merged 14 commits intoAzure:mainfrom
JialinHuang803:remove-apiver-policy

Conversation

@JialinHuang803
Copy link
Copy Markdown
Member

Summary

This pull request improves the handling of the api-version parameter, especially for long-running operations (LROs) and paged operations. It removes the use of custom pipeline policies for injecting api-version and instead ensures the parameter is consistently and correctly added to URLs as needed, and set the default apiVersion value at the operations instead of the client context.

Changes

Remove the custom apiVersion pipeline policy:

  • Updates the client context and operation generation logic to remove all references to the pipeline policy and instead pass apiVersion explicitly where needed.
    • Modifies the LRO and paging helpers to accept and use an optional apiVersion parameter, ensuring that polling and paging requests include the correct API version if provided.
  • Adds logic to extract api-version from URLs where necessary, such as when restoring pollers.

Move the default apiVersion value to the operation level:

  • Updates parameter serialization logic to handle api-version parameters with default values, ensuring defaults are respected when the parameter is not provided.
  • Updates the apiVersion parameter to be optional in the generated client context interfaces.

@JialinHuang803 JialinHuang803 marked this pull request as ready for review January 27, 2026 05:40
@JialinHuang803 JialinHuang803 linked an issue Jan 27, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Member

@qiaozha qiaozha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good, just two minor comments.

Comment thread packages/typespec-ts/static/static-helpers/pagingHelpers.ts Outdated
Comment thread packages/typespec-ts/static/static-helpers/pollingHelpers.ts
Copy link
Copy Markdown
Member

@joheredi joheredi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I'd like to change the dummy url though. See my comment, other than that I have no concerns with merging this PR

* @returns - the URL with the api-version query parameter set
*/
function addApiVersionToUrl(url: string, apiVersion: string): string {
const urlObj = new URL(url, "https://microsoft.com");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we require a base url parameter?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I encountered azureCore.spec.ts this case, and an error was thrown if it's not an absolute URL. So I added it in case the input URL is relevant.

Comment thread packages/typespec-ts/static/static-helpers/pagingHelpers.ts Outdated
/** The API version to use for this operation. */
/** Known values of {@link KnownVersions} that the service accepts. */
apiVersion: string;
apiVersion?: string;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we have this diff? a bug?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context.apiVersion can be undefined since we don't apply the client default value to the client context anymore if user don't set the value.

@qiaozha qiaozha added HRLC p0 priority 0 labels Jan 28, 2026
@JialinHuang803 JialinHuang803 enabled auto-merge (squash) January 30, 2026 01:06
@JialinHuang803 JialinHuang803 merged commit fc2b6e1 into Azure:main Jan 30, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

HRLC p0 priority 0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

remove client api version policy

4 participants