Skip to content

refactor(sdk-python): replace exception name matching with type checks#376

Open
Gujiassh wants to merge 2 commits intoalibaba:mainfrom
Gujiassh:refactor/python-exception-type-checks
Open

refactor(sdk-python): replace exception name matching with type checks#376
Gujiassh wants to merge 2 commits intoalibaba:mainfrom
Gujiassh:refactor/python-exception-type-checks

Conversation

@Gujiassh
Copy link
Contributor

@Gujiassh Gujiassh commented Mar 6, 2026

Summary

  • replace string-based exception name checks in the Python SDK exception converter with direct isinstance() checks against generated UnexpectedStatus types and httpx exception classes
  • keep behavior unchanged while making the converter more robust to refactors and easier to reason about
  • add regression coverage for generated UnexpectedStatus and HTTPStatusError conversion paths

Testing

  • uv run pytest tests/test_converters_and_error_handling.py -q
  • uv run pyright src/opensandbox/adapters/converter/exception_converter.py

@jwx0925
Copy link
Collaborator

jwx0925 commented Mar 9, 2026

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Delightful!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ninan-nn
ninan-nn previously approved these changes Mar 9, 2026
Copy link
Collaborator

@ninan-nn ninan-nn left a comment

Choose a reason for hiding this comment

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

LGTM

@ninan-nn
Copy link
Collaborator

ninan-nn commented Mar 9, 2026

plz rebase main and fix ruff check.

@Gujiassh Gujiassh force-pushed the refactor/python-exception-type-checks branch from 3646c87 to 59d824e Compare March 10, 2026 01:14
@Gujiassh
Copy link
Contributor Author

Done - rebased onto the latest main and cleaned up the ruff import ordering. I also reran local ruff, pyright, and pytest for the Python SDK, and they're all green now. Thanks for the heads-up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants