Skip to content

feat: initial Python client for the ISMS API#2

Merged
unidoc-ahall merged 3 commits into
masterfrom
feat/python-client
Jul 7, 2026
Merged

feat: initial Python client for the ISMS API#2
unidoc-ahall merged 3 commits into
masterfrom
feat/python-client

Conversation

@unidoc-ahall

Copy link
Copy Markdown
Contributor

First version of the official Python client + CI + repo policy.

What

IsmsClient over the /api/v1 REST surface — register namespaces (suppliers, risks, incidents, corrective actions, tasks, references, documents, identity), typed exceptions, and a Go-CLI-compatible env-file loader. from_env() for zero-boilerplate setup.

Endpoint correctness

Every route was checked against the server's actual handlers before opening this, and four mismatches were corrected:

  • IdentityGET /me (there is no /whoami).
  • DocumentsGET /documents/all (tree) + GET /documents/:id/body; there is no GET /documents or /documents/:id.
  • list() → unwraps the {"data":[...], "total":N} envelope, so .list() yields a list, not the envelope dict.
  • Org selectionX-Organization-UUID (a UUID); a subdomain URL already scopes the org server-side.

CI + policy (mirrors the isms repo)

  • tests.yml — ruff + pytest across Python 3.10–3.13; public-repo security model (pull_request only, read-only token).
  • Governance: CONTRIBUTING / CODE_OF_CONDUCT / SECURITY + issue templates.
  • scripts/apply-branch-protection.sh — master policy (PR + 1 review, required CI, signed commits, linear history, no force-push) applied via the GitHub API; run at the end of setup, version-controlled instead of hand-clicked.

Tests

pytest (21) + ruff green, via responses-mocked HTTP; a regression test for each of the four fixes above.

The official Python client (isms.sh), matching the isms CLI / Go client surface.

- IsmsClient + from_env (env vars / ISMS_ENV file loader compatible with the Go
  CLI's loadEnvFile; documented precedence).
- Register namespaces: suppliers, risks, incidents, corrective actions, tasks,
  cross-entity references, documents, and identity (whoami → /me).
- list() unwraps the {"data":[...]} envelope the API returns.
- Typed exceptions (auth / not-found / validation / http) + py.typed marker.
- Endpoints verified against the live /api/v1 routes; tests use responses.

Closes #1
- GitHub Actions (tests.yml): ruff lint + pytest on Python 3.10–3.13, public-repo
  security model (pull_request only, read-only token, no secrets).
- Governance mirrored from the isms repo: CONTRIBUTING, CODE_OF_CONDUCT, SECURITY,
  and issue templates (bug / feature / docs + config).
- scripts/apply-branch-protection.sh: version-controlled enforcement of the master
  policy (PR + review, required CI, signed commits, linear history) via the GH API.
- gitignore temp/ (local scratch / torun scripts).
@unidoc-ahall unidoc-ahall requested a review from unidoc-alip July 7, 2026 12:34

@unidoc-alip unidoc-alip left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Solid first version of the Python client. The endpoint correctness claims in the PR description were cross-checked directly against the ISMS Go server routes and handlers (/me, /documents/all + /body + /search, the list envelope shape, X-Organization-UUID, and every resource's CRUD and status routes), and all of them check out. A security pass over the HTTP client, env loader, CI workflow, and branch protection script found no exploitable issues.

Two things worth fixing before or shortly after merge:

1. [should-fix] ReferenceResource.list()src/isms/client.py:291

Takes entity_type and entity_id as optional keyword arguments, but the server's handleListReferences handler requires both query params and returns 400 Bad Request if either is missing. Confirmed by reading the handler directly (internal/isms/api/api_references.go, lines 36-41). As written, client.references.list() with no arguments will always raise IsmsValidationError.

Suggested fix — make both parameters required instead of optional:

def list(self, entity_type: str, entity_id: str) -> list[dict]:
    return _as_list(
        self._http.get(
            "/v1/references",
            params={"type": entity_type, "id": entity_id},
        )
    )

Worth adding a test that calling list() with no arguments fails fast (TypeError) instead of round-tripping to the server to discover the 400.

2. [nit] _resolve_base_urlsrc/isms/client.py:46

Decides whether a URL already has an /api suffix using a raw substring check: "/api/" in url + "/". Reproduced the bug directly:

_resolve_base_url("https://demo.isms.sh/api/docs")
# -> "https://demo.isms.sh/api/docs"  (unchanged, should have /api appended)

The substring /api/ happens to appear in the path even though it's not the actual API mount point, so a deployment reverse-proxied under a path containing /api/ would silently get the wrong base URL. This is a minor, low-likelihood edge case given the documented usage is always a bare subdomain URL, but it's cheap to fix by checking only the last path segment instead of doing a substring search:

def _resolve_base_url(url: str) -> str:
    url = url.rstrip("/")
    if url.rsplit("/", 1)[-1] == "api":
        return url
    return url + "/api"

Verified this produces the same result for the documented cases (https://demo.isms.sh becomes .../api, and https://demo.isms.sh/api is left unchanged) while correctly handling the /api/docs-style edge case.

Neither issue blocks merge; the first is a real bug a user will hit on first use of the unfiltered call, the second is a defensive fix for an edge case that is unlikely in practice.

Addresses Alip's review on PR #2:

- ReferenceResource.list() now takes entity_type and entity_id as required
  arguments. The server's handleListReferences returns 400 without both, so a
  bare list() call would always fail against the server — now it fails fast with
  TypeError at the call site instead of round-tripping.
- _resolve_base_url matches only the final path segment (== 'api') instead of a
  raw '/api/' substring, so a deployment proxied under a path like /api/docs no
  longer silently skips appending the API mount.

Tests: references.list() with no args raises TypeError; it sends both params when
given; _resolve_base_url covers the documented cases and the /api/docs edge.
@unidoc-ahall

Copy link
Copy Markdown
Contributor Author

Thanks @unidoc-alip — both done.

1. references.list() required args — made entity_type and entity_id required positional args, exactly as you suggested; a bare references.list() now raises TypeError at the call site instead of round-tripping to the server's 400. Added a test for that + one asserting both params go on the wire.
2. _resolve_base_url — now checks only the last path segment (rsplit('/', 1)[-1] == 'api'), so /api/docs-style proxy paths get /api appended correctly. Added test_resolve_base_url_only_matches_last_segment covering the documented cases + that edge.

ruff + pytest green (24).

@unidoc-alip unidoc-alip left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good to me

@unidoc-ahall unidoc-ahall merged commit 4ef96be into master Jul 7, 2026
5 checks passed
@unidoc-ahall unidoc-ahall deleted the feat/python-client branch July 7, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants