Fix Python Quick Start example to compile with current SDK#1310
Conversation
The Quick Start snippet in python/README.md called create_session(model=...) without the required keyword-only on_permission_request argument, so running it raised: TypeError: CopilotClient.create_session() missing 1 required keyword-only argument: 'on_permission_request'. Add the PermissionHandler import and pass on_permission_request=PermissionHandler.approve_all. Apply the same one-line fix to the API Reference snippet directly below, which had the identical bug. Fixes #1079 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Python README examples so CopilotClient.create_session() includes the required permission handler argument.
Changes:
- Imports
PermissionHandlerin the Quick Start snippet. - Passes
on_permission_request=PermissionHandler.approve_allin two READMEcreate_session()examples.
Show a summary per file
| File | Description |
|---|---|
python/README.md |
Updates Python documentation snippets to include the required permission handler argument. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
| async with await client.create_session( | ||
| on_permission_request=PermissionHandler.approve_all, | ||
| model="gpt-5", | ||
| ) as session: |
Cross-SDK Consistency Review ✅This PR only modifies Consistency check across other SDKs:
The fix is accurate and necessary — Python enforces keyword-only arguments at runtime, so the missing No cross-SDK consistency issues to flag. The change is approved from a consistency perspective.
|
The Quick Start snippet in
python/README.mdwas the first thing a new user copy-pasted, and it failed immediately with:CopilotClient.create_sessiondeclareson_permission_requestas a required keyword-only parameter (no default), but the snippet calledcreate_session(model="gpt-5")without it.Fix
python/README.mdonly — no source changes.from copilot.session import PermissionHandlerand passon_permission_request=PermissionHandler.approve_all.PermissionHandlerwas already imported there, but the call still omitted the kwarg, so it would have failed the same way.Kept
model="gpt-5"instead of changing the model name — the bug was the missing permission handler, not the model, and"gpt-5"is what the rest of the README uses.Validation
python -m py_compileandast.parse.inspect.signature.bind()against the realCopilotClient.create_sessionsignature (no missing required kwargs, no unknown kwargs).TypeErroragainst the unpatched snippet and confirmed the patched snippet no longer raises it.Fixes #1079