feat!: sample-import record ids are strings - #22
Merged
Conversation
The API serialises record ids as strings: they are 18-digit values, far past what JavaScript represents exactly, so an integer on the wire is corrupted in any browser client. Python has no such limit, but declaring these int while the wire says string left pydantic silently coercing and the tests asserting a shape the server no longer sends. Fixtures now send strings, so the round trip is proven rather than assumed. --job-id still rejects non-numeric input: ids are always digits, and a non-numeric one would fail deep inside the server's query building. BREAKING CHANGE: SampleImportJob.id, .sample_ids and .execution_id are now str, and 'samples import --json' emits them quoted.
str.isdigit() is true for digits from any script, so the guard admitted values it was written to stop. '²³' passes isdigit() but is not parseable as a number at all, reaching the server exactly as the guard exists to prevent. '٤٢' is worse: it passes, and now that ids travel as strings it is forwarded verbatim rather than normalised, silently addressing a different job than the one asked for. Requiring ASCII alongside isdigit() also rejects '-5' and ' 42', which the previous int() parse accepted. Job ids are never negative or space-padded, so that tightening is intended.
…ids yet Pydantic does not coerce int -> str in lax mode, so SampleImportJob raised ValidationError against any deployment still on the integer-id API — the normal state of an on-prem install caught between client and server upgrades. Sets coerce_numbers_to_str on the model; Python's unbounded ints mean no precision is lost converting an 18-digit id. frozen moves into model_config alongside it.
job.sample_ids is list[str] now that the API serialises record ids as strings; str() on each element was a no-op.
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
1 participant
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.
Companion to goodwright/flow-api#267 (FLOW-725). Merge this first — that PR's acceptance fixtures expect these types.
Why
The Flow API now serialises v2 record ids as JSON strings. Ids are 18-digit
BigIntegerprimary keys, far past what JavaScript represents exactly, so an integer on the wire is silently corrupted in any browser client.Python has no such limit, so pydantic's lax mode was quietly coercing the new strings back to
intand nothing broke. But the declared types then lied about the wire, and the tests asserted a shape the server no longer sends.What changed
SampleImportJob.id,.sample_idsand.execution_idare nowstr. There was already precedent in this file:PubMedIdis astrdocumented as "a bare integer written as a string".--job-idstill rejects non-numeric input, and now also rejects non-ASCII digits.'٤٢'.isdigit()isTruewhileint('٤٢')is42, so a naive digit check would have forwarded the raw Arabic-Indic string as the job id — a silently wrong value. Regression test included.coerce_numbers_to_str=Truemeans an older server that still sends integer ids continues to parse. A client is upgraded independently of the deployment it talks to, and without this a released 0.12.0 pointed at a not-yet-upgraded install would fail everysamples import/import-statuscall with an uncaughtValidationError. An 18-digit id round-trips exactly.BREAKING CHANGE
SampleImportJob.id,.sample_idsand.execution_idare nowstr, andsamples import --jsonemits them quoted. Version0.11.1→0.12.0.Consumers reading those fields out of
--jsonwithjqwill see"42"where they saw42.Verified
pytest tests/unit/→ 395 passed.