Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file removed .coverage
Binary file not shown.
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ jobs:
- name: Run mypy
run: python -m mypy glpi_python_client

# The sync client tree is generated from the async one and checked in.
# This catches a _sync/ left stale by an edit to _async/. It does NOT
# catch a token collision -- that is deterministic, so regenerating
# reproduces it and the diff stays clean. test_unasync_codegen.py
# covers that case.
- name: Check the generated sync tree is up to date
run: python unasync_build.py --check

- name: Build documentation
run: python -m sphinx -W --keep-going -b html docs docs/_build/html

Expand Down
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,7 @@ CLAUDE.md
docs/superpowers/*

# Large generated GLPI API contract (kept locally, not tracked)
docs/glpi_api_contract.json
docs/glpi_api_contract.json
# Coverage data: rewritten by every test run (and by the venv .pth hook).
.coverage
.coverage.*
10 changes: 10 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ repos:
types: [python]
pass_filenames: false

# Fails on a _sync/ tree left stale by an edit to _async/. Runs at
# commit time rather than pre-push so the generated tree is never
# committed out of step with its source.
- id: unasync-check
name: generated sync tree is up to date
entry: python unasync_build.py --check
language: system
types: [python]
pass_filenames: false

- id: pytest-coverage
name: pytest with coverage (>=95%)
entry: python -m pytest -m "not integration" --cov=glpi_python_client --cov-fail-under=95 -q
Expand Down
132 changes: 132 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

### Fixed

- **A `Major` priority ticket made the whole search fail.** GLPI's priority
scale has six levels; the published contract advertises five, and
`GlpiPriority` followed the contract. Since `GetTicket.priority` is typed
with that enum and validation runs per record, a single escalated ticket
anywhere in a result set raised `ValidationError` and took the entire
query down with it — most likely to bite exactly the reporting queries
that filter on high priority. `GlpiPriority.MAJOR = 6` is now defined.
The five existing members keep their identifiers, so stored filters are
unaffected. `urgency` and `impact` genuinely do stop at 5 and now accept a
value GLPI will never send, which is harmless in the direction that
matters.

- **The statistics layer sent GLPI v1 field names to the v2 API, which
silently ignored them and returned unfiltered results.** v2 drops a
`filter=` conjunct whose field it does not recognise, honours the rest,
Expand Down Expand Up @@ -154,6 +166,62 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

### Changed

- **`AsyncGlpiClient` now performs real non-blocking I/O.** It was a facade
that wrapped each synchronous method in `asyncio.to_thread`, so "async"
meant "blocking call on a worker thread". It is now genuinely
asynchronous, built on `httpx.AsyncClient`, with no thread pool and no
executor. The `executor` constructor keyword is gone, as is
`AsyncBridge`.
- The two clients are now one codebase. `glpi_python_client/_async/` is
hand-written and `glpi_python_client/_sync/` is generated from it by
`unasync_build.py`, committed, and diffed in CI. Endpoint logic exists
exactly once, so the two surfaces cannot drift.
- This deletes the six hand-written async override modules the bridge
forced into existence — including the 500-line `_statistics_async.py`,
which duplicated the most intricate logic in the package with no test
asserting the two copies agreed.
- Aggregating helpers keep their concurrency through a shared `gather`
helper that is `asyncio.gather` on the async surface and sequential
evaluation on the generated one, written once at the call site.
- **Public imports are unchanged**: `from glpi_python_client import
GlpiClient, AsyncGlpiClient` still works. Code importing private
module paths (`glpi_python_client.clients.*`, `glpi_python_client.auth.*`)
must add the tree segment, e.g.
`glpi_python_client._sync.clients.commons._transport`.
- **Breaking: the HTTP transport moved from `requests` to `httpx`.**
`requests` and `urllib3` are no longer dependencies. The v2 transport, the
legacy v1 session, and the OAuth token manager were swapped together in a
single change because they share `_http.py`; splitting them would have left
the shared code validated only by tests exercising the old transport.
Behaviour is preserved, which took three deliberate corrections where the
two libraries disagree and the difference is silent:
- **Query parameters with a `None` value are dropped**, as `requests` did.
`httpx` encodes them as a valueless `key=`, and GLPI treats an empty
filter or search value as *match everything* — so the swap would have
silently widened queries rather than leaving them unconstrained.
- **`bytes` and `bool` parameter values keep their previous rendering**
(`b"x"` → `x`, `True` → `True`). `httpx` would emit the Python repr
`b'x'` and a lowercase `true`.
- **Redirects are still followed.** `requests` follows them by default and
`httpx` does not, so a followed redirect would have started surfacing as
a bare 3xx response.
- **Breaking: network-level faults now raise `GlpiTransportError`** (or its
`GlpiTimeoutError` subclass) instead of propagating the HTTP library's own
exception. This completes the promise the previous release documented:
catching `GlpiError` is now sufficient for the library's whole failure
surface, and you never need to import the HTTP library. The originating
exception is attached as `__cause__`. Code doing
`except requests.RequestException` should now catch `GlpiTransportError`.
Note it does *not* inherit `ValueError` — nothing was passed in wrongly and
no value came back.
- The retry predicates were retargeted onto this library-owned type in the
same change. This is the failure mode that made the swap risky: the
exception trees of the two libraries are completely disjoint, so a
predicate left naming the old one stops matching and **every retry
silently disappears** — no error, no warning, and a green test suite.
Naming a type the library itself raises makes that impossible to
reintroduce. A mutation test confirms the suite catches it: reverting the
predicate fails 7 tests across all three transports.
- **Breaking:** a persistent 5xx now raises `GlpiServerError` instead of
`tenacity.RetryError`. The retry decorators gained `reraise=True`. Code
doing `except tenacity.RetryError` and digging out
Expand All @@ -173,6 +241,70 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
module and its `remote_error_message` helper are removed. It had no
library call sites, and `reraise=True` leaves it nothing to unwrap.

### Performance

- **`sniffio` is now a dependency, and the async client is ~2.6x faster at
wide fan-out because of it.** `httpcore` decides whether it is running
under asyncio or trio by probing for `sniffio` on every async request,
falling back to `"asyncio"` when the import fails. Nothing in the
dependency chain required it — `httpx` pulls in `anyio`, and `anyio` 4.14
dropped `sniffio` — so a fresh install had no `sniffio`, and because
Python never caches a failed import, every single request re-walked
`sys.path` doing filesystem stats. Measured against a local server with
50 ms latency, a fan-out of 128 took **3354 ms without `sniffio` and
1304 ms with it**. `pip check` reports no broken requirements either way,
which is why this went unnoticed: nothing declares the package, nothing
imports it, and the only symptom is that every request is slower.

- **Bounding a wide fan-out is now a documented requirement, not a
suggestion.** `httpcore` rescans its entire connection pool on every
request assignment, calling `has_expired()` per connection — profiled at
9040 such calls for a 64-request fan-out. The cost is quadratic in the
width of the fan-out and it saturates the event loop, so server-observed
concurrency *falls* as the fan-out widens. Raising `httpx.Limits` does not
help. At a fan-out of 16 against a 50 ms server, an unbounded
`gather` took 350 ms while the same work capped at 16 with an
`asyncio.Semaphore` took 108 ms. See "Bounding concurrency" in the user
guide. This is a property of `httpx` 0.28 / `httpcore` 1.0.9, which are
the current releases; there is no version to upgrade to.

### Documentation

- **The documentation still described the deleted bridge**, in the places
users actually read: the README, the user guide, the API reference, the
package docstring, and two skills all said `AsyncGlpiClient` wraps each
synchronous method into a coroutine dispatched to a worker thread via
`asyncio.to_thread`. None of that has been true since the codegen
rewrite. The worst of it was live API: the user guide's *Custom thread
pools* section and step 8 of `glpi-client-setup` documented an
`executor=` constructor argument that no longer exists, so anyone
copying either example got a `TypeError`. That section is now
*Bounding concurrency* and shows an `asyncio.Semaphore`, which is what
actually bounds a fan-out now.
- **Fourteen docstring cross-references pointed at modules the rewrite had
renamed or deleted** (`clients.async_client`, `clients.sync_client`,
`custom._ticket_context_async`). Nothing caught them: Sphinx runs with
`nitpicky` off, so an unresolvable target renders as plain text rather
than failing the build, and these private modules are not autodoc'd in
the first place. `tests/test_docstring_references.py` now resolves every
qualified reference in the package against the live modules.
- **The generated sync tree documented itself in terms of the async one.**
unasync repoints imports, where `_async` is its own NAME token, but a
dotted path inside a docstring is a single string token and passes
through untouched — so 47 cross-references in the shipped sync client
pointed into `_async/`. The diff gate is blind to this for the same
reason it is blind to a token collision: the omission is deterministic,
so regeneration reproduces it and the diff stays clean. `unasync_build`
now repoints the qualified prefix, and a test asserts the generated tree
never names `_async` at all.
- `TicketContextMixin` claimed its five calls ran sequentially and that an
async override fanned them out, contradicting both the code and its own
method docstring. The development guide still described the deleted
`_ticket_context_async.py` / `_statistics_async.py` and the retired
`test_parity.py` / `test_async_selfcall_guard.py` suites.
- The `requests` intersphinx mapping is removed; it survived the transport
swap and made every docs build fetch an inventory nothing referenced.

### Unchanged (deliberately)

- Retry semantics: 5xx retried 3 times with a 3-second fixed wait, 4xx never
Expand Down
11 changes: 7 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,13 @@ python -m sphinx -W --keep-going -b html docs docs/_build/html
## Design Guidelines

- Keep API calls behind `GlpiClient` / `AsyncGlpiClient` methods. Add
new endpoints to a sync endpoint mixin only; `AsyncGlpiClient`
exposes them as coroutines automatically through `AsyncBridge`. Only
add a dedicated async override when the method needs concurrent
fan-out via `asyncio.gather`.
new endpoints to the async endpoint mixin under
`glpi_python_client/_async/`, then run `python unasync_build.py` to
regenerate `glpi_python_client/_sync/` and commit both. Never edit
`_sync/` by hand; CI regenerates it and fails on any difference. For
concurrent fan-out use `gather` from `_async/_concurrency.py` rather
than `asyncio.gather` directly, so the generated sync code stays
correct.
- Prefer field-validated Pydantic models for request and response payloads.
- Avoid organization-specific category, entity, or profile defaults in the
library core.
Expand Down
32 changes: 19 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ back to HTML for outgoing payloads.
It currently focuses on ticket-centric workflows and exposes two high-level
clients built on top of the GLPI v2 REST API:

- `GlpiClient` — synchronous, blocking client (single source of truth for
endpoint behaviour).
- `AsyncGlpiClient` — asynchronous facade that wraps every synchronous
method into a coroutine and dispatches it to a worker thread.
- `GlpiClient` — synchronous, blocking client.
- `AsyncGlpiClient` — asynchronous client doing real non-blocking I/O on the
event loop.

Neither wraps the other. The async tree is hand-written and the sync one is
generated from it, so the two surfaces cannot drift apart.

Note that all integration tests using this package are made on GLPI 11.
I cannot make any guarantee of the behaviour on previous versions.
Expand Down Expand Up @@ -106,15 +108,19 @@ variables.
### Sync or async?

Both clients expose the exact same endpoint surface and accept the same
constructor arguments. The async client is a thin facade that wraps each
synchronous method into a coroutine dispatched to a worker thread via
`asyncio.to_thread` (or a caller-supplied `concurrent.futures.Executor`).
A shared `threading.Lock` serialises OAuth token acquisition so concurrent
`asyncio.gather(...)` fan-outs cannot race. Pick `GlpiClient` for plain
scripts, CLI tools, and synchronous services; pick `AsyncGlpiClient` when
your application already runs an event loop or when you need concurrent
fan-out (the aggregated `get_ticket_context` and per-ticket
`get_task_statistics` helpers use `asyncio.gather` on the async client).
constructor arguments, because both are generated from one source: the
async tree is hand-written and the sync one is produced from it by a build
step that strips `async`/`await`. `AsyncGlpiClient` performs real
non-blocking I/O on the event loop — there is no worker thread and no
executor — and `GlpiClient` performs real blocking I/O with no coroutine
scheduling. An auth lock serialises OAuth token acquisition on both
surfaces, so concurrent fan-outs cannot race.

Pick `GlpiClient` for plain scripts, CLI tools, and synchronous services;
pick `AsyncGlpiClient` when your application already runs an event loop or
when you need concurrent fan-out (the aggregated `get_ticket_context` and
per-ticket `get_task_statistics` helpers overlap their calls there, and run
them one after another on the sync client).

## Documentation

Expand Down
21 changes: 9 additions & 12 deletions docs/api_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ Clients
-------

The package exposes two clients with identical endpoint surfaces. The
synchronous one is the single source of truth for endpoint behaviour;
the asynchronous one wraps each synchronous method into a coroutine.
asynchronous one is hand-written and the synchronous one is generated
from it, so neither wraps the other and the two cannot drift apart.

.. autoclass:: GlpiClient
:members:
Expand All @@ -24,10 +24,6 @@ the asynchronous one wraps each synchronous method into a coroutine.
:inherited-members:
:show-inheritance:

.. autoclass:: glpi_python_client.clients.commons._async_bridge.AsyncBridge
:members:
:show-inheritance:

Exceptions
----------

Expand All @@ -37,12 +33,13 @@ unusable response body derive from :class:`GlpiError`.
:class:`GlpiProtocolError` also inherit :class:`ValueError` for backwards
compatibility with releases that raised bare ``ValueError``.

This is not the library's entire failure surface. Network-level faults
(connection failures, DNS errors, timeouts) still propagate as
``requests`` exceptions today: :class:`GlpiTransportError` and
:class:`GlpiTimeoutError` are reserved for that case but are not raised
until a future httpx transport swap. A handful of sites also
deliberately still raise bare ``RuntimeError`` or ``TypeError`` instead
Network-level faults (connection failures, DNS errors, timeouts) are
raised as :class:`GlpiTransportError`, or its :class:`GlpiTimeoutError`
subclass for a timeout, with the underlying transport exception attached
as ``__cause__``. Catching :class:`GlpiError` is therefore sufficient for
the library's failure surface -- you never need to import the HTTP
library. A handful of sites do still deliberately raise bare
``RuntimeError`` or ``TypeError`` instead
of a library type, so existing ``except RuntimeError`` / ``except
TypeError`` code keeps working. See :ref:`error-handling` in the user
guide for the full picture, including which methods raise which type.
Expand Down
1 change: 0 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,4 @@ def _read_project_version() -> str:

intersphinx_mapping = {
"python": ("https://docs.python.org/3", None),
"requests": ("https://requests.readthedocs.io/en/latest/", None),
}
Loading
Loading