-
Notifications
You must be signed in to change notification settings - Fork 292
[Http Client] Client resolution #7658
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
base: main
Are you sure you want to change the base?
Conversation
❌ There is undocummented changes. Run The following packages have changes but are not documented.
Show changes |
You can try these changes here
|
The following parameters and default instantiation strategy apply when no explicit `@clientInitialization` decorator is present: | ||
|
||
- **Endpoint, Credential, API Version, and Subscription ID parameters** are automatically included as described above. | ||
- **Initialization mode** is set to `individual`, where each client is constructed independently. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value should be undefined
. This ask is from JS, which is aims to separate whether to generate the CTOR for the sub-client.
|
||
1. **Check for explicit decorators** | ||
|
||
- If **any** `@client` or `@operationGroup` appears in the spec, then: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we try to remove the @operationGroup
and reuse @client
to define the sub client? It is on the plan for TCGC but not yet implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the rule could be @client
under any @client
will be the sub clients, others are top-level client.
|
||
- Every namespace annotated with `@client` becomes a **root client**. | ||
- Within each root client, nested namespaces and interfaces annotated with `@operationGroup` are resolved as sub-clients | ||
- Can be configured to hoist all nested operations and skip implicit sub-clients (e.g., `flattenSubClients: true`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default is true
, right?
- Within each service client: | ||
|
||
- All operations declared directly in it become client methods. | ||
- Each nested namespace (recursively, to any depth) is turned into a sub-client, preserving the namespace hierarchy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about nested interface. TCGC also treat it as sub-client.
|
||
- All operations declared directly in it become client methods. | ||
- Each nested namespace (recursively, to any depth) is turned into a sub-client, preserving the namespace hierarchy. | ||
- Unless client resolution is configured to hoist operations to the top-level client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this logic control by flattenSubCleints
flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
@@ -0,0 +1,28 @@ | |||
# Custom name policy for clients |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate with get-clients.md
?
4. **Prune empty clients** | ||
|
||
- Any client or sub-client that ends up with **no** methods _and_ **no** sub-clients is discarded (ignored). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will @clientLocation
move to the http-client?
* @param options The options to use when resolving clients. | ||
* @returns The resolved clients and any diagnostics. | ||
*/ | ||
export function resolveClients( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the logic does not care about versioning. Then how could TCGC get the client with the version mutated types?
return clientState.get(container)!; | ||
} | ||
|
||
const client: Client = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic seems from previous version of TCGC when there is not @scope
or @clientLocation
. I'm not sure how we should deal with these two decorators in TCGC. Maybe mutate the type graph, then call this typekit? If use mutate, then do versioning mutator first or do the scope/clientLocation mutator first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe versioning mutators need to be run before enganing the http-client library. @typespec/versioning already handles it so there is no need for http-client to know about versioning
This PR promotes the core client resolution logic from TCGC into
@typespec/http-client
. I have added a docs section underpackages/http-client/docs
with conceptual descriptions of the algorithms used for calculation (based on TCGC docs) and practical how-to showing how to use the new APIsThis PR includes:
@client
Follow up work:
@clientName