Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
233 lines (179 loc) · 9.84 KB

CONTRIBUTING.md

File metadata and controls

233 lines (179 loc) · 9.84 KB
Raw

Contributing

This is a Forward Networks Field Integration, maintained by the field team. Issues and pull requests are welcome and handled on a best-effort basis; it is not routed through Forward Support.

Setup

uv sync --all-groups
uv run pre-commit install

Checks

uv run ruff check . && uv run ruff format --check .
uv run mypy
uv run pytest
uv run python scripts/check_generated.py
uv run mkdocs build --strict

What is generated

Much of this repository is generated, and CI fails if the checked-in output is stale. Do not edit these by hand:

Path Generated by
spec/forward-openapi.yaml scripts/sync_spec.py, from the Forward monorepo
spec/forward-openapi-3.1.json scripts/downconvert_spec.py
src/forward_sdk/_generated/models.py scripts/gen_models.py
src/forward_sdk/_generated/operations.py scripts/gen_operations.py
src/forward_sdk/_async/services/_generated/ scripts/gen_services.py
src/forward_sdk/_sync/, tests/_sync/ scripts/unasync.py

Updating to a new Forward release

uv run python scripts/sync_spec.py --fwd-root ~/src/fwd

That vendors the description, regenerates everything, and prints which operations were added, removed or deprecated. That list is the starting point for a changelog entry.

Then run the checks. tests/spec/test_coverage.py names any operation that gained or lost an implementation, so nothing slips through silently.

Knowing when to do it

Nothing tells you. CI cannot: the Forward monorepo is not available to it, so scripts/check_generated.py only proves the committed spec and the committed code agree, never that the spec is current. A description that is a year stale passes every check in this repository.

So check by hand when you touch this, and before a release:

jq -r .fwd_commit spec/SPEC_SOURCE.json
git -C ~/src/fwd rev-parse HEAD

Different hashes mean the vendored description may be behind. Re-running the sync is cheap and idempotent against an unchanged checkout, so run it rather than reason about whether anything relevant changed.

spec/unpublished.yaml has no upstream to sync from at all. Those shapes are observed rather than generated, so a Forward release can change one without anything here noticing. The scheduled live workflow exists for that; see running the live tests.

Writing async, getting sync

Anything that performs I/O is written once, as async, under src/forward_sdk/_async/ and tests/_async/. The synchronous client is generated from it:

uv run python scripts/unasync.py

Never edit _sync directly. If a construct is awkward to translate mechanically, it probably belongs in the pure layers (_ops, nqe) that both clients already share.

Adding an operation

Most operations need no code: they are generated from the description. Write a builder by hand only when the shape is unusual, such as multipart upload or a body assembled from several arguments.

  1. Add a builder in src/forward_sdk/_ops/, decorated @op("theOperationId"). The decorator binds it to the operation table, so its method, path and dispatch cannot disagree with the spec.
  2. Add a service method under src/forward_sdk/_async/services/, and list the group in HAND_WRITTEN_TAGS in scripts/gen_services.py so it is not also generated.
  3. Run scripts/unasync.py, then the tests.

Unpublished endpoints

Endpoints outside Forward's published description belong in spec/unpublished.yaml and are marked stability="unpublished". Document them in docs/unpublished.md. They carry no compatibility promise, so keep them isolated and say so in the docstring.

A blind spot this suite has

Most of these tests assert that the SDK produces the strings and requests it produces. That catches regressions, and it cannot catch a shared wrong assumption: when a parser and its fixtures are written together, they agree with each other whether or not they agree with Forward.

Two shipped defects came from exactly that. where.py emitted field contains "value" for months of development, and NQE has no contains operator, so every tag-scoped predicate would have been rejected. A query's commit id was read from a flat key Forward never sends, so every query resolved by path silently ran against head. Both had tests. Both tests passed.

What found them was a second implementation. Running this client and an existing one against the same fake Forward and diffing the request sequences surfaces disagreements that neither suite can see alone, because the disagreement does not exist inside either one.

So when you change how a request is built or a response is read, prefer evidence from outside this repository: a recorded payload, a live call, or another client that talks to the same endpoint. The wire shapes in spec/unpublished.yaml exist for this reason, and tests/spec/test_unpublished_shapes.py runs the parsers against them rather than against fixtures written alongside the parsers.

The first run against a real instance found seven defects in code that had a green suite, including a predicate helper that could never have worked, a repository method that failed on every call, and a Forward AI response that failed to parse at all. None was subtle. All were assumptions.

Another client agreeing with you is not evidence

The advice above has a hole, and one defect went through it. Both plugins this SDK replaces stripped a fields wrapper from NQE result rows, so the SDK did too. Forward never sends that wrapper. Its serializer holds a row in a field named fields internally and writes the row's own entries at the top level, so the envelope exists only in the server's own representation, and the API description documents bare rows on every result endpoint.

Three codebases held the belief at once, and one of them had encoded it in seventeen fixtures. Every one of those fixtures was written by the same people who wrote the parser it fed. Consensus between consumers is not independent confirmation, because consumers copy each other, and this SDK copied both.

The stripping was not merely useless. select {fields: {...}} yields a row whose single key is fields, indistinguishable on the wire from the envelope it was mistaken for, so those rows were silently rewritten and could not be recovered downstream.

Rank your evidence. The server's source and its generated description outrank a live call, which outranks a recorded payload, which outranks another client's behaviour. Another client tells you what someone else believed. Only the first three tell you what Forward does. When a shape is not in the description, say in the docstring which of these you checked, so the next person knows whether they are reading an observation or an inheritance.

Running the live tests

export FORWARD_URL=https://fwd.app
export FORWARD_USERNAME=... FORWARD_PASSWORD=... FORWARD_NETWORK_ID=...
uv run pytest -m live

They are skipped without credentials, so they never block an ordinary run. If you change a predicate builder, a response parser, or anything under spec/unpublished.yaml, run them before you believe the unit tests.

.github/workflows/live.yml runs them weekly against a configured instance and opens an issue when they fail, so a Forward release changing an unpublished shape is reported here rather than discovered in someone's sync. It needs four repository secrets, FORWARD_URL, FORWARD_USERNAME, FORWARD_PASSWORD and FORWARD_NETWORK_ID, and skips cleanly without them.

The class TestShapesWithNoCiBackstop is the point of that schedule. Each test in it pins a shape that has been wrong at least once and that no other check in this repository can catch. Add to it whenever a defect turns out to have been a wrong assumption about what Forward sends, and say in the docstring what the wrong assumption cost.

Be careful what you write on a shared instance. Verification should be read-only where it can be, anything created should be removed in a finally, and the account should be left as it was found.

Tests

  • tests/unit/ -- pure functions, no I/O.
  • tests/_async/ -- behaviour, through httpx.MockTransport. Generated into tests/_sync/, so both clients are covered by one set of assertions.
  • tests/spec/ -- coverage and conformance against the vendored description.
  • tests/live/ -- against a real instance, skipped without credentials.

Releasing

  1. Update __version__ in src/forward_sdk/_version.py and CHANGELOG.md.
  2. Tag vX.Y.Z. The release workflow refuses to publish if the tag and the version disagree, then publishes via PyPI Trusted Publishing.

PyPI setup, already done

Trusted Publishing is configured, so tagging is all a release needs. No long-lived credential lives in this repository. The publisher registered at https://pypi.org/manage/project/forward-sdk/settings/publishing/ is:

Field Value
Owner forwardnetworks
Repository forward-python-sdk
Workflow name release.yml
Environment name pypi

The pypi environment exists under Settings → Environments and can take required reviewers if publishing should need a human approval.

Releases 0.1.0 through 0.1.4 predate this and were uploaded manually with an API token, so their files carry no attestations; PyPI does not add them to files already published, and re-running a release job over an existing version does not backfill them. Attestations start with the first release published through the workflow.

If a future release fails with invalid-publisher, the publisher record no longer matches the claims the workflow presents. The failure text prints those claims, so compare them against the table above rather than against the workflow file: workflow_ref must end in release.yml, and environment must read pypi. The build and test job runs first either way, so a tag is verified even when the upload is refused.