Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a cache for parameters/responses/operations #6030

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ArcturusZhang
Copy link
Member

@ArcturusZhang ArcturusZhang commented Feb 17, 2025

Fixes #6029
Contributes to #5920

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Feb 17, 2025
@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

@typespec/http-client-csharp

@@ -184,7 +184,6 @@ export function createModel(sdkContext: CSharpEmitterContext): CodeModel {
Type: parameterType,
Location: RequestLocation.Uri,
IsApiVersion: parameter.isApiVersionParam,
IsResourceParameter: false,
Copy link
Member Author

@ArcturusZhang ArcturusZhang Feb 17, 2025

Choose a reason for hiding this comment

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

I removed this parameter because in everywhere of our emitter, we are assigning false to this property, and in all the places in MGC.
In autorest.csharp, the only chance that this parameter is assigned to true is in the converter that we convert from swagger, when the parameter has a certain x-ms- extension.
Therefore I think we should not maintain it any more.

This property has never been assigned to true in tspCodeModel.json, therefore it is fine that the InputParameter class in autorest.csharp could keep this property, but we no longer need it here.

@@ -456,7 +456,6 @@ private static IReadOnlyList<ParameterProvider> BuildSpreadParametersForModel(In
false,
false,
false,
false,
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO - this part is not quite correct to do.
We should not be constructing any input types since we are out of the input library construction.
Instead, this part I think we should

  1. Build the spread model and get its corresponding ModelProvider.
  2. "Spread" it into parameters from ModelProvider and directly get its corresponding ParameterProvider.

I will create an issue later to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[http-client-csharp] move the cache of parameters/responses into the CSharpEmitterContext
2 participants