[tcgc] one client multiple services#3460
Conversation
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
commit: |
…cgc/multiServiceOneClient
| this.apiVersion !== undefined && | ||
| this.apiVersion !== "latest" && | ||
| this.apiVersion !== "all" && | ||
| !this.__packageVersions.includes(this.apiVersion) | ||
| !serviceVersions.includes(this.apiVersion) |
There was a problem hiding this comment.
This config could only be used for one service. How could this work for multiple service's version?
There was a problem hiding this comment.
We may need to think about how to specific multiple service's api version in the config.
…cgc/multiServiceOneClient
…cgc/multiServiceOneClient
…cgc/multiServiceOneClient
| // operations directly under the group | ||
| const operations = [...group.type.operations.values()]; | ||
|
|
||
| // For interfaces that extend other interfaces, also collect operations from the source interfaces |
There was a problem hiding this comment.
Is it a bug from TypeSpec? This seems a hack to get back the operations that raw type graph does not have. If it only happens when having versioning, I doubt if it is a versioning mutator issue.
| const diagnostics = createDiagnosticCollector(); | ||
|
|
||
| const packageVersions = context.getPackageVersions(); | ||
| const packageVersions = context.getApiVersions(); |
There was a problem hiding this comment.
This will fail for example loading since different service have different version.
| // Find the service that has versioning information | ||
| // In single-service scenarios, this should be the service with @versioned | ||
| // In multi-service without @useDependency, use the first service with versions | ||
| let targetService = services[0].type; | ||
| for (const service of services) { | ||
| const versions = getVersions(context.program, service.type)[1]; | ||
| if (versions) { | ||
| targetService = service.type; | ||
| break; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
So, for multiple service, we always need to define @client with parent to build the hierarchy?
If so, here we could just keep the original logic. I'm not get the point to find a versioned service for multi-service.
If not, I think we could generate paralleled clients with these services.
| reportDiagnostic(this.program, { | ||
| code: "multiple-services-require-use-dependency", | ||
| format: { services: services.map((s) => s.type.name).join(", ") }, | ||
| target: services[0].type, | ||
| }); |
There was a problem hiding this comment.
If not meet the case, shall we return directly?
| const cachedVersions = this.__serviceToVersions.get(service); | ||
| if (cachedVersions?.length) { | ||
| return cachedVersions; | ||
| } | ||
|
|
||
| const versions = getVersions(program, service)[1]?.getVersions(); | ||
| if (!versions) { | ||
| this.__packageVersions = []; | ||
| return this.__packageVersions; | ||
| return []; | ||
| } |
There was a problem hiding this comment.
For services that without versioning, we should also save the cache.
| if (!parent && client.kind === "SdkClient" && client.parent) { | ||
| const parentRaw = context.__rawClientsOperationGroupsCache?.get(client.parent) as | ||
| | SdkClient | ||
| | undefined; | ||
| parent = context.__clientTypesCache?.find((c) => c.__raw === parentRaw) as | ||
| | SdkClientType<TServiceOperation> | ||
| | undefined; | ||
| } |
There was a problem hiding this comment.
I don't prefer to mix the client hierarchy build into the SDK type creation section. It's better to do it in the cache process.
| } | ||
| } | ||
|
|
||
| // Check for multi-service scenario with @useDependency |
There was a problem hiding this comment.
I'm wondering why the API parameter no longer poped up from the operation level's parameter for multi-service cases? I believe only the top-level client should have special logic.
| return diagnostics.wrap(sdkPackage); | ||
| } | ||
|
|
||
| function createClients<TServiceOperation extends SdkServiceOperation>( |
There was a problem hiding this comment.
Can we move this client hierarchy build process into cache builder section. I prefer after we get the client hierarchy info, all the following calculation does not have special logic for multi-service scenario. The only exception could be API version cache builder.
| ); | ||
| }); | ||
|
|
||
| it("one client from multiple services", async () => { |
There was a problem hiding this comment.
There are diagnostic error for these test cases. See playground. We need to change rule of invalid-initialized-by.
This reverts commit a872ce2.
fixes #3536, fixes #3537