refactor: convert to proper open-source Python SDK package#3
Conversation
- Restructure as installable package (src layout, pyproject.toml with hatchling) - Add full type annotations on all public methods and parameters - Add custom exception hierarchy (HelpdeskError, APIError, ConfigurationError) - Add TicketState and NoteType enums with PEP 561 py.typed marker - Replace print() with logging, remove pretty_print_response - Rename type params to note_type/ticket_type (avoid builtin shadowing) - Rename search_ticket to search_tickets with snake_case keyword-only params - Make cert_path optional (defaults to standard verification) - Add comprehensive test suite (25 tests, 93% coverage) - Add CI pipeline (GitHub Actions: pytest, ruff, mypy across Python 3.10-3.13) - Add PyPI publish workflow via trusted publishers (OIDC) - Rewrite README with badges, API reference, and install instructions - Add CONTRIBUTING.md and CHANGELOG.md https://claude.ai/code/session_012jDYBKYfKc83cBjHEjh51n
There was a problem hiding this comment.
Pull request overview
This PR refactors the original single-file helpdesk.py script into a properly structured, installable Python SDK package (lansweeper-helpdesk). It adds type annotations, a proper exception hierarchy, enums, tests, CI/CD workflows, and packaging configuration, while removing anti-patterns (bare print() calls, rate-limiting delays, etc.).
Changes:
- Restructures the code into an installable
src/lansweeper_helpdesk/package with separate modules for the client, exceptions, and types. - Adds a comprehensive test suite (
tests/) usingpytestandresponses, along withconftest.pyfixtures. - Introduces packaging infrastructure (
pyproject.toml,README.md,CHANGELOG.md,CONTRIBUTING.md, CI/publish workflows).
Reviewed changes
Copilot reviewed 15 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/lansweeper_helpdesk/client.py |
Core API client, refactored with full type annotations, proper exception raising, and logging |
src/lansweeper_helpdesk/exceptions.py |
Custom exception hierarchy (HelpdeskError, APIError, ConfigurationError, TicketNotFoundError) |
src/lansweeper_helpdesk/types.py |
StrEnum enums for TicketState/NoteType and APIResponse type alias |
src/lansweeper_helpdesk/__init__.py |
Package public surface (__all__, version, top-level imports) |
src/lansweeper_helpdesk/py.typed |
PEP 561 marker for typed package |
tests/test_client.py |
Tests for all public API methods and _request edge cases |
tests/test_types.py |
Tests for enum values and string behaviour |
tests/conftest.py |
Shared fixtures for HelpdeskAPI instances and temporary cert file |
tests/__init__.py |
Makes tests/ a package |
pyproject.toml |
Hatch-based build config, dependencies, dev extras, ruff/mypy settings |
README.md |
New user-facing README replacing the old readme.md |
CHANGELOG.md |
Structured changelog documenting breaking changes from the old API |
CONTRIBUTING.md |
Developer contribution guide |
helpdesk.py |
Deleted (replaced by the src/ package) |
requirements.txt |
Deleted (replaced by pyproject.toml dependencies) |
readme.md |
Deleted (replaced by README.md) |
config/config.example.json |
Added example config file |
.gitignore |
Standard Python .gitignore |
.github/workflows/ci.yml |
CI pipeline across Python 3.10–3.13 |
.github/workflows/publish.yml |
OIDC-based PyPI publish workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| class TicketNotFoundError(APIError): | ||
| """Raised when a requested ticket does not exist.""" |
There was a problem hiding this comment.
TicketNotFoundError is exported in all and documented in the README error-handling table as "Raised when a requested ticket does not exist," but it is never raised anywhere in the client code. This makes the documentation misleading — callers who catch TicketNotFoundError will never see it triggered.
Either the client should raise TicketNotFoundError in get_ticket() when the API returns a 404 (or an equivalent "not found" response), or the exception and its documentation should be removed until the raising logic is implemented.
| class TicketNotFoundError(APIError): | |
| """Raised when a requested ticket does not exist.""" |
| ticket_id: str, | ||
| text: str, | ||
| email: str, | ||
| note_type: str = "Public", |
There was a problem hiding this comment.
The add_note() method's note_type parameter defaults to the plain string "Public" rather than NoteType.PUBLIC. Since the NoteType enum was introduced specifically to represent the valid values for this parameter, using NoteType.PUBLIC as the default would be more consistent and would self-document the accepted values directly in the signature.
|
|
||
|
|
There was a problem hiding this comment.
The StrEnum backport for Python 3.10 (line 13) does not override generate_next_value. The real StrEnum in Python 3.11+ returns the member name lowercased when auto() is used. Without overriding generate_next_value, using auto() on Python 3.10 with this backport would silently produce integer values instead of lowercase strings. While the current enums use explicit string values and are not affected, adding a generate_next_value override would make the backport a more complete and safer substitute to prevent subtle bugs if auto() is ever used in the future.
| def _generate_next_value_(self, name, start, count, last_values): | |
| """Return the member name lowercased when using auto().""" | |
| return name.lower() |
| __all__ = [ | ||
| "HelpdeskAPI", | ||
| "APIError", | ||
| "ConfigurationError", | ||
| "HelpdeskError", | ||
| "TicketNotFoundError", | ||
| "NoteType", | ||
| "TicketState", | ||
| "__version__", | ||
| ] |
There was a problem hiding this comment.
APIResponse is the return type of every public method in the SDK (create_ticket, get_ticket, search_tickets, etc.), so users who want to type-annotate their own code using the SDK's return types need this alias. It is defined in types.py but not included in init.py's all or imported there, requiring users to do from lansweeper_helpdesk.types import APIResponse instead of from lansweeper_helpdesk import APIResponse. Exporting it alongside TicketState and NoteType would make the public API surface complete for typed consumers.
No description provided.