Repository navigation
feat: add skip_preflight and REJECTED job status (0.47.0) - #353
Closed
gavinchen2025 wants to merge 4 commits into
Closed
gavinchen2025 wants to merge 4 commits into
gavinchen2025 wants to merge 4 commits into
Conversation
Two gaps kept the client behind the server on the job pre-flight check. REJECTED job status ------------------- JobStatus and JobProcessingStatus were closed enums without REJECTED, so get_job on a rejected job raised, and one rejected job in a page failed the whole list_jobs call, since the page is validated as a single list. Both enums now carry REJECTED plus a _missing_ hook that preserves an unrecognised server-side value verbatim, so the next status the server adds is not a client break. The hook cannot come from the spec - a generated enum is closed - so both files stay in .genignore with a guard test. skip_preflight -------------- Added to CreateWorkflow and UpdateWorkflow to opt a workflow out of the check, and to WorkflowInformation to read the setting back. Modelled exactly on reprocess_all: OptionalNullable[bool] = UNSET on the request models, so an unset value is omitted rather than sent as false, which is what makes the server's "omit means unchanged" contract work on update. Sending false opts back in. BodyCreateJob takes it as a plain argument. The spec types request_data as a string and describes the payload only in contentSchema, so there is no model to generate; the field folds into that JSON instead, and carries no MultipartFormMetadata so it never becomes a form part of its own. An explicit argument wins over a value already in the string. The fold splices rather than re-encoding, so every other byte of the caller's payload survives verbatim - a json.loads/json.dumps round trip preserves values but not their representation. It runs on assignment and through model_copy as well as construction: the field is invisible on the wire, so a dropped fold would reach the server as nothing at all and raise nothing, running the job with preflight enabled while the caller believes otherwise. Re-running it on an already-correct payload is a no-op. Also exports Unset from unstructured_client.types. UNSET was exported but not its type, leaving no public way to test for unset-ness - pydantic deep-copies the default per instance, so an identity check against the singleton silently fails open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
Resolutions:
_version.py kept 0.47.0. main released 0.46.2 after this branch point;
0.47.0 supersedes it. Both __version__ and __user_agent__.
CHANGELOG.md kept both entries, 0.47.0 above 0.46.2. 0.46.2 shipped to
RELEASES.md PyPI, so dropping its entries would lose release history.
RELEASES.md is oldest-first, so 0.46.2 precedes 0.47.0 there.
.genignore honored main's deletion.
test_regen... honored main's deletion.
The Speakeasy generation contract is being wound down, so the
file protects nothing. Its removal also makes every test that
asserted membership in it vacuous, so those are gone with it.
What was NOT dropped: the behaviour those guards sat next to.
test_skip_preflight_models.py carries the nine tests that
verify what the models actually do - REJECTED and the enums'
forward tolerance, skip_preflight on all four models, the
Unset export, and the request_data fold across construction,
assignment and model_copy, including its byte-preservation and
no-op properties. Those are the real cover: if a regeneration
or a careless edit drops any of it, these fail. The deleted
assertions only checked that a path appeared in a text file.
Verified on the merged tree: 295 unit + 89 contract passing, pylint 10.00/10,
mypy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Setting skip_preflight back to None -- by assignment or model_copy -- left the
previously folded "skip_preflight": true inside request_data. The object then
said "no preference" while the wire still carried true, so the job ran with
preflight off after the caller had cleared the flag. Silent, and in the
permissive direction.
The early return lumped Unset and None together. That is right when
request_data is untouched caller bytes, but after a fold it no longer is. The
two are different statements and now behave differently:
Unset never mentioned -> passthrough, byte for byte, including a
skip_preflight the caller wrote into the
JSON themselves
None explicitly no preference -> clears a folded key; the explicit
argument wins, as it already does for
True and False
Widening the existing branch to cover both would have introduced the opposite
bug: stripping a key the caller wrote and never asked us to touch. None on a
non-JSON payload also stays silent rather than raising -- nothing was folded
in, so there is nothing to undo.
Also makes the splice preserve surrounding whitespace. The comment claimed it
"leaves every other byte exactly as handed to us" while rstrip() discarded the
indentation before the closing brace and any trailing newline. The splice now
keeps content, the gap before the brace, and anything after it as separate
pieces and reassembles them, so the claim is true.
README: correct the enum containment note. It said `some_string in
shared.JobStatus` is true for any string; that holds only on 3.12. CPython
changed __contains__ twice, so across the supported versions the same
expression raises TypeError on 3.11, returns True on 3.12 and returns False on
3.13. Verified on all three. __members__ is stable everywhere and stays the
recommendation. Also documents the None-clears-versus-unset-passthrough split
above.
No behaviour change to the enums themselves: forward tolerance on
deserialization works identically on 3.11, 3.12 and 3.13.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
The None path removed the folded key by re-encoding the whole document, so
clearing the flag rewrote unrelated configuration: 1e5 became 100000.0, 1.50
became 1.5, and precision past a double was lost. The insert path goes out of
its way to preserve the caller's bytes, so having the clear path discard them
contradicted the code's own contract.
The fold writes a fixed fragment, so clearing can be its exact inverse rather
than a re-encode. `_drop_skip_preflight` removes that fragment from the raw
text, searching from the right because the fold always appends after existing
content. The candidate is parsed and compared against the expected payload
before being accepted, so a fragment that happens to match a *nested*
`skip_preflight` is rejected instead of silently corrupting the document. A
caller who wrote the key with their own spacing matches no fragment and falls
back to a re-encode, which is correct but not byte-preserving.
Fold-then-clear is now a true round trip: '{\n "x": 1e5\n}\n' comes back
identical, as do a 21-digit decimal, non-ASCII, an empty object and '{ }'.
Narrow in practice - it needs a hand-written request_data, since a json.dumps
payload is already normalized before the SDK sees it, plus a non-canonical
number and a fold-then-clear transition. Fixed for the consistency reason
above rather than the precision one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.
Re-trigger cubic
Contributor
|
Closing in favor of a smaller replacement for DTPL-696. The replacement uses the SDK's existing open-enum mechanism ( |
CyMule
added a commit
that referenced
this pull request
Oct 21, 2026
) ## What Jobs the new preflight check rejects no longer break the SDK. Workflows can also opt out of preflight from the SDK. **Before**, after a job is rejected: ``` client.jobs.list_jobs(...) → ValidationError: 'REJECTED' is not a valid JobStatus (whole page lost) client.jobs.get_job(...) → ValidationError ``` **After:** ``` job.status → JobStatus.REJECTED job.status → "SOME_FUTURE_STATUS" # a status this SDK version doesn't know comes back as a plain str instead of raising details.node_stats[0].failure_category → PreflightFailureCategory.AUTH_INVALID_CREDENTIAL details.node_stats[0].failure_reason → "The AWS Access Key Id you provided does not exist." ``` - `JobStatus` and `JobProcessingStatus` gain `REJECTED`. `JobInformation.status` and `JobDetails.processing_status` also accept unknown values. Known values are still enum members, so `job.status == JobStatus.COMPLETED` and `.value` behave as before. - `JobNodeDetails` gains `failure_category` (`Union[PreflightFailureCategory, str]`) and `failure_reason`, so a caller can see why a job was rejected. The server sets them only on the node that failed preflight; before this, pydantic dropped them. - `skip_preflight` is now on `CreateWorkflow`, `UpdateWorkflow` (unset = unchanged) and `WorkflowInformation`. For a one-off `create_job`, the API defines the flag inside the `request_data` JSON. The docs now say to set it there; the SDK does not rewrite that string. - Releases as **0.47.0**. ## Why The SDK models used to be regenerated from the API spec every night. That generator has been dead since January (removed in #349). Since then, new server values reach the SDK only when someone adds them by hand. `REJECTED` didn't get added, and a closed enum turns a new value into a hard failure. This PR also adds a way to catch that kind of drift before a customer does (DTPL-696 acceptance criterion 4): ``` weekly CI → fetch live OpenAPI specs → compare with SDK models → new missing field/enum value → open/update a GitHub issue (a failed download fails the run but opens no issue) ``` - `make check-spec-drift` runs the check locally. Gaps we already know about are recorded in `scripts/spec_drift_baseline.json` (20 schemas today, e.g. `JobInformation.processing_status`, the `teradata` connector type). The check fails only on new drift, or on a baseline entry that has since been covered. - `CONTRIBUTING.md` documents how to resolve a drift issue. Replaces #353, which got here through a custom `_missing_` enum hook and code that spliced bytes into `request_data`. This version uses a plain pydantic union and leaves `request_data` alone. ## Notes for consumers - Type checkers now see these two status fields as `Union[JobStatus, str]` / `Union[JobProcessingStatus, str]`. Code that reads `.value` or `.name` on them needs to handle the `str` case. Runtime behavior for known statuses is unchanged. - Follow-up, not in this PR: the SDK's other "open" enums (connector types, partition `Strategy`, etc.) use `OpenEnumMeta` / `validate_open_enum`. On Python 3.12+ those return a raw `str` even for known values, and they emit pydantic serializer warnings. The long-term fix is to move them to the same union pattern used here. That changes their runtime types, so it belongs in its own PR. ## Validation - `make test-unit`: 419 passed. `make test-contract`: 72 passed. The new job/workflow contract tests pass on Python 3.11, 3.12 and 3.13, and they fail without the model changes. - `make lint` is clean. `make check-spec-drift` passes against the live specs. - Two rounds of Codex review. Round 1 found that enums declared inline on a property escaped the drift check; that is fixed and has a test. Round 2 was clean. - cubic: three findings fixed (no drift issue on a fetch failure, no duplicate issues from overlapping runs, changelog wording). Re-review clean, and full CI is green, including integration on 3.11–3.13. ## Maintenance validation (2026-10-19, keepalive base refresh) - Synchronized main through its 0.46.5 keepalive release while retaining the authored 0.47.0 version and user-agent. Manual conflict resolution touched only the changelog and version file; main runtime and test changes merged automatically. - Local validation: 419 unit tests and 71 contract tests passed (one existing expected unit failure). Four localhost socket tests needed bind permission and passed without source changes. - Exact maintenance-head CI: https://github.com/Unstructured-IO/unstructured-python-client/actions/runs/39329146842. ## Review follow-up (2026-10-20) - Added `failure_category` / `failure_reason` to `JobNodeDetails` with a new `PreflightFailureCategory` enum, and dropped both from the drift baseline. A new contract test covers a known category, an unknown category (plain `str`) and a passing node (both null); it passes on 3.11, 3.12 and 3.13. - Removed the `JobStatus` / `JobProcessingStatus` entries from `overlay_client.yaml`. Nothing reads the overlay since the generator was removed, and `x-speakeasy-unknown-values` would describe the `OpenEnumMeta` style, not the union shipped here. - `make test-unit` 419 passed, `make test-contract` 72 passed, `make lint` clean, `make check-spec-drift` clean against the live specs. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two gaps kept the client behind the server on the job pre-flight check.
REJECTED job status
JobStatus and JobProcessingStatus were closed enums without REJECTED, so get_job on a rejected job raised, and one rejected job in a page failed the whole list_jobs call, since the page is validated as a single list. Both enums now carry REJECTED plus a missing hook that preserves an unrecognised server-side value verbatim, so the next status the server adds is not a client break.
The hook cannot come from the spec - a generated enum is closed - so both files stay in .genignore with a guard test.
skip_preflight
Added to CreateWorkflow and UpdateWorkflow to opt a workflow out of the check, and to WorkflowInformation to read the setting back. Modelled exactly on reprocess_all: OptionalNullable[bool] = UNSET on the request models, so an unset value is omitted rather than sent as false, which is what makes the server's "omit means unchanged" contract work on update. Sending false opts back in.
BodyCreateJob takes it as a plain argument. The spec types request_data as a string and describes the payload only in contentSchema, so there is no model to generate; the field folds into that JSON instead, and carries no MultipartFormMetadata so it never becomes a form part of its own. An explicit argument wins over a value already in the string.
The fold splices rather than re-encoding, so every other byte of the caller's payload survives verbatim - a json.loads/json.dumps round trip preserves values but not their representation. It runs on assignment and through model_copy as well as construction: the field is invisible on the wire, so a dropped fold would reach the server as nothing at all and raise nothing, running the job with preflight enabled while the caller believes otherwise. Re-running it on an already-correct payload is a no-op.
Also exports Unset from unstructured_client.types. UNSET was exported but not its type, leaving no public way to test for unset-ness - pydantic deep-copies the default per instance, so an identity check against the singleton silently fails open.