Move client creation to functions#583
Move client creation to functions#583brettcannon wants to merge 8 commits intogithub:maingithub/copilot-sdk:mainfrom
Conversation
…ique to CLI launching
The `cli_client()` and `network_client()` functions separate out the two ways to launch the client to simplify things. The initializer for `CopilotClient` is made private by making the config objects it accepts private to keep the public API simple.
There was a problem hiding this comment.
Pull request overview
This pull request refactors the Python SDK's client creation API to use factory functions instead of direct instantiation. The change introduces copilot.cli_client() and copilot.network_client() as the primary ways to create client instances, simplifying the public API by making configuration classes private.
Changes:
- Introduces factory functions
cli_client()andnetwork_client()for creating client instances - Replaces
CopilotClientOptionsTypedDict with private dataclasses_CLIClientConfigand_NetworkClientConfig - Updates all test scenarios, samples, and documentation to use the new API pattern
Reviewed changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/copilot/client.py | Adds factory functions cli_client() and network_client(), refactors __init__ to accept config objects, converts options dict to instance attributes |
| python/copilot/types.py | Replaces CopilotClientOptions TypedDict with private _CLIClientConfig and _NetworkClientConfig dataclasses |
| python/copilot/init.py | Exports new factory functions cli_client and network_client, adds LogLevel to imports |
| python/test_client.py | Updates all tests to use new factory functions |
| python/e2e/*.py | Updates E2E tests to use copilot.cli_client() |
| python/e2e/testharness/context.py | Updates test harness to use copilot.cli_client() |
| python/samples/chat.py | Updates sample to use copilot.cli_client() |
| test/scenarios/*/python/main.py | Updates all 25 scenario tests to use new factory functions |
| python/README.md | Comprehensive documentation update explaining the new factory functions and their parameters |
| docs/*.md | Updates all documentation examples to use the new API pattern |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@brettcannon I'm not sure about this one. Having separate APIs for different connection types makes this different from all the other language SDKs. There are many different constructor args so it's not obvious to me that this is the most important one that we should bifurcate the APIs based on. Plus the notions of What would be the payoff from doing this split? |
Simpler parameters based on needs and what Python folks are more used to. The config object approach is just not common in the Python community (hence why the builder pattern isn't either) as Python's signature support makes building up arguments less necessary. Plus, you can fake config objects with dicts (like how the API currently does it). But I understand if you're concerned about support overhead. Let me think about whether config objects with class method helpers or everything as parameters but without overloads makes the most sense (probably the former as that at least supports type checkers, although I don't know if having class methods so users aren't forced to construct intermediate config objects will be a maintenance concern like you had with the overloads). |
|
Agree with @SteveSandersonMS on this one. Would really like to hold consistent patterns as much as possible since it is easier for agents to pick up and do the right thing when people are integrating it using an agentic coding tool. |
The
cli_client()andnetwork_client()functions separate out the two ways to launch the client to simplify things. The initializer forCopilotClientis made private by making the config objects it accepts private to keep the public API simple.