From 2abfc5885f04beed3829a43459bb8e2eb364cf0e Mon Sep 17 00:00:00 2001 From: Martin Husbyn Date: Mon, 17 Aug 2026 19:46:57 +0100 Subject: [PATCH 1/4] feat!: sample-import record ids are strings 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. --- flowbio/cli/_samples.py | 9 ++++---- flowbio/v2/samples.py | 9 ++++---- setup.py | 2 +- source/cli.rst | 4 ++-- tests/unit/cli/test_samples.py | 28 +++++++++++------------ tests/unit/v2/test_samples.py | 42 +++++++++++++++++----------------- 6 files changed, 48 insertions(+), 46 deletions(-) diff --git a/flowbio/cli/_samples.py b/flowbio/cli/_samples.py index c51b093..159a365 100644 --- a/flowbio/cli/_samples.py +++ b/flowbio/cli/_samples.py @@ -271,10 +271,11 @@ def _configure_import(import_parser: argparse.ArgumentParser) -> None: def _job_id(value: str) -> SampleImportJobId: - try: - return SampleImportJobId(int(value)) - except ValueError: - raise argparse.ArgumentTypeError(f"job id must be an integer, got {value!r}") from None + # Ids are strings on the wire but always digits; a non-numeric one would + # fail deep inside the server's query building rather than at the boundary. + if not value.isdigit(): + raise argparse.ArgumentTypeError(f"job id must be an integer, got {value!r}") + return SampleImportJobId(value) def _configure_import_status(import_status: argparse.ArgumentParser) -> None: diff --git a/flowbio/v2/samples.py b/flowbio/v2/samples.py index 709e01e..c7b3197 100644 --- a/flowbio/v2/samples.py +++ b/flowbio/v2/samples.py @@ -147,9 +147,10 @@ class MultiplexedUpload(BaseModel, frozen=True): ) -SampleImportJobId = NewType("SampleImportJobId", int) +SampleImportJobId = NewType("SampleImportJobId", str) """The identifier of a sample-import job, as returned by -:meth:`SampleResource.import_samples`.""" +:meth:`SampleResource.import_samples`. A bare integer written as a string — +the API serialises record ids as strings so browsers do not round them.""" SampleImportStatus = Literal["RUNNING", "COMPLETED", "FAILED"] @@ -217,11 +218,11 @@ class SampleImportJob(BaseModel, frozen=True): accessions: list[str] = Field( default_factory=list, description="The accessions submitted with this job, in submission order.", ) - sample_ids: list[int] = Field( + sample_ids: list[str] = Field( default_factory=list, description="The created samples' ids, corresponding to ``accessions`` once the job has completed.", ) - execution_id: int | None = Field( + execution_id: str | None = Field( default=None, description="The pipeline execution backing this job, if one was created.", ) error: str | None = Field( diff --git a/setup.py b/setup.py index bab87fd..6d007c2 100644 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ setup( name="flowbio", - version="0.11.1", + version="0.12.0", description="A client for the Flow API.", long_description=long_description, long_description_content_type="text/markdown", diff --git a/source/cli.rst b/source/cli.rst index be722d8..9119e4c 100644 --- a/source/cli.rst +++ b/source/cli.rst @@ -460,7 +460,7 @@ otherwise the standard mapping above. Started import job 42 for 2 accession(s) (status: RUNNING). Check progress with 'flowbio samples import-status --job-id 42'. $ flowbio samples import --sheet ./accessions.csv --json - {"id": 42, "status": "RUNNING", "created": "2024-04-05T19:34:38Z", "started": null, "finished": null, "accessions": ["ERR1160845", "ERR10677146"], "sample_ids": [], "execution_id": null, "error": null} + {"id": "42", "status": "RUNNING", "created": "2024-04-05T19:34:38Z", "started": null, "finished": null, "accessions": ["ERR1160845", "ERR10677146"], "sample_ids": [], "execution_id": null, "error": null} ``samples import-status`` ~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -517,7 +517,7 @@ mapping above. Job 42: COMPLETED (finished 2024-04-05 19:38:20 UTC). Sample ids: 101, 102. $ flowbio samples import-status --job-id 42 --json - {"id": 42, "status": "COMPLETED", "created": "2024-04-05T19:34:38Z", "started": "2024-04-05T19:34:40Z", "finished": "2024-04-05T19:38:20Z", "accessions": ["ERR1160845", "ERR10677146"], "sample_ids": [101, 102], "execution_id": 7, "error": null} + {"id": "42", "status": "COMPLETED", "created": "2024-04-05T19:34:38Z", "started": "2024-04-05T19:34:40Z", "finished": "2024-04-05T19:38:20Z", "accessions": ["ERR1160845", "ERR10677146"], "sample_ids": ["101", "102"], "execution_id": "7", "error": null} ``api get`` ~~~~~~~~~~~ diff --git a/tests/unit/cli/test_samples.py b/tests/unit/cli/test_samples.py index 23ef963..cfe8ef3 100644 --- a/tests/unit/cli/test_samples.py +++ b/tests/unit/cli/test_samples.py @@ -953,14 +953,14 @@ def _job_json( execution_id: int | None = 7, ) -> dict: return { - "id": job_id, + "id": str(job_id), "status": status, "created": 1700000000, "started": 1700000001 if status != "RUNNING" else None, "finished": 1700000002 if status in ("COMPLETED", "FAILED") else None, "accessions": accessions, - "sample_ids": sample_ids or [], - "execution_id": execution_id, + "sample_ids": [str(sample_id) for sample_id in sample_ids or []], + "execution_id": None if execution_id is None else str(execution_id), "error": error, } @@ -1010,14 +1010,14 @@ def test_json_document_reports_job_fields( document = json.loads(result.stdout) assert result.stdout.count("\n") == 1 assert document == { - "id": 42, + "id": "42", "status": "RUNNING", "created": "2023-11-14T22:13:20Z", "started": None, "finished": None, "accessions": ["ERR1"], "sample_ids": [], - "execution_id": 7, + "execution_id": "7", "error": None, } @@ -1391,7 +1391,7 @@ def test_reports_running_job(self, run_cli) -> None: def test_running_job_with_started_reports_when_it_started(self, run_cli) -> None: respx.get(f"{SAMPLE_IMPORTS_URL}/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, "status": "RUNNING", "created": 1700000000, + "id": "42", "status": "RUNNING", "created": 1700000000, "started": 1700000001, "finished": None, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), @@ -1408,7 +1408,7 @@ def test_running_job_with_started_reports_when_it_started(self, run_cli) -> None def test_started_with_non_utc_offset_is_reported_in_utc(self, run_cli) -> None: respx.get(f"{SAMPLE_IMPORTS_URL}/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, "status": "RUNNING", "created": 1700000000, + "id": "42", "status": "RUNNING", "created": 1700000000, "started": "2024-04-05T19:34:38+02:00", "finished": None, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), @@ -1425,7 +1425,7 @@ def test_started_with_non_utc_offset_is_reported_in_utc(self, run_cli) -> None: def test_started_with_no_offset_is_treated_as_utc(self, run_cli) -> None: respx.get(f"{SAMPLE_IMPORTS_URL}/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, "status": "RUNNING", "created": 1700000000, + "id": "42", "status": "RUNNING", "created": 1700000000, "started": "2024-04-05T19:34:38", "finished": None, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), @@ -1442,7 +1442,7 @@ def test_started_with_no_offset_is_treated_as_utc(self, run_cli) -> None: def test_naive_started_is_reported_as_utc_in_json_too(self, run_cli) -> None: respx.get(f"{SAMPLE_IMPORTS_URL}/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, "status": "RUNNING", "created": 1700000000, + "id": "42", "status": "RUNNING", "created": 1700000000, "started": "2024-04-05T19:34:38", "finished": None, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), @@ -1460,7 +1460,7 @@ def test_naive_started_is_reported_as_utc_in_json_too(self, run_cli) -> None: def test_non_utc_offset_started_is_reported_as_utc_in_json_too(self, run_cli) -> None: respx.get(f"{SAMPLE_IMPORTS_URL}/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, "status": "RUNNING", "created": 1700000000, + "id": "42", "status": "RUNNING", "created": 1700000000, "started": "2024-04-05T19:34:38+02:00", "finished": None, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), @@ -1478,7 +1478,7 @@ def test_non_utc_offset_started_is_reported_as_utc_in_json_too(self, run_cli) -> def test_running_job_falls_back_to_created_when_not_started(self, run_cli) -> None: respx.get(f"{SAMPLE_IMPORTS_URL}/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, "status": "RUNNING", "created": 1700000000, + "id": "42", "status": "RUNNING", "created": 1700000000, "started": None, "finished": None, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), @@ -1596,14 +1596,14 @@ def test_json_document_matches_job_shape(self, run_cli) -> None: document = json.loads(result.stdout) assert result.stdout.count("\n") == 1 assert document == { - "id": 42, + "id": "42", "status": "COMPLETED", "created": "2023-11-14T22:13:20Z", "started": "2023-11-14T22:13:21Z", "finished": "2023-11-14T22:13:22Z", "accessions": ["ERR1"], - "sample_ids": [101], - "execution_id": 7, + "sample_ids": ["101"], + "execution_id": "7", "error": None, } diff --git a/tests/unit/v2/test_samples.py b/tests/unit/v2/test_samples.py index ae57639..5324320 100644 --- a/tests/unit/v2/test_samples.py +++ b/tests/unit/v2/test_samples.py @@ -1063,7 +1063,7 @@ class TestImportSamples: def test_posts_imports_and_parses_job(self) -> None: route = respx.post(f"{DEFAULT_BASE_URL}/v2/sample-imports").mock( return_value=httpx.Response(HTTPStatus.CREATED, json={ - "id": 42, + "id": "42", "status": "RUNNING", "created": 1700000000, "started": None, @@ -1081,7 +1081,7 @@ def test_posts_imports_and_parses_job(self) -> None: ]) assert result == SampleImportJob( - id=SampleImportJobId(42), + id=SampleImportJobId("42"), status="RUNNING", created=1700000000, started=None, @@ -1097,7 +1097,7 @@ def test_posts_imports_and_parses_job(self) -> None: def test_sends_accession_and_sample_type(self) -> None: route = respx.post(f"{DEFAULT_BASE_URL}/v2/sample-imports").mock( return_value=httpx.Response(HTTPStatus.CREATED, json={ - "id": 1, "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1"], + "id": "1", "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), ) @@ -1116,7 +1116,7 @@ def test_sends_accession_and_sample_type(self) -> None: def test_sends_optional_fields_when_present(self) -> None: route = respx.post(f"{DEFAULT_BASE_URL}/v2/sample-imports").mock( return_value=httpx.Response(HTTPStatus.CREATED, json={ - "id": 1, "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1"], + "id": "1", "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), ) @@ -1151,7 +1151,7 @@ def test_sends_optional_fields_when_present(self) -> None: def test_empty_string_optional_fields_are_omitted_not_the_required_ones(self) -> None: route = respx.post(f"{DEFAULT_BASE_URL}/v2/sample-imports").mock( return_value=httpx.Response(HTTPStatus.CREATED, json={ - "id": 1, "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1"], + "id": "1", "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1"], "sample_ids": [], "execution_id": None, "error": None, }), ) @@ -1172,7 +1172,7 @@ def test_empty_string_optional_fields_are_omitted_not_the_required_ones(self) -> def test_sends_multiple_imports_in_one_request(self) -> None: route = respx.post(f"{DEFAULT_BASE_URL}/v2/sample-imports").mock( return_value=httpx.Response(HTTPStatus.CREATED, json={ - "id": 1, "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1", "ERR2"], + "id": "1", "status": "RUNNING", "created": 1700000000, "accessions": ["ERR1", "ERR2"], "sample_ids": [], "execution_id": None, "error": None, }), ) @@ -1210,30 +1210,30 @@ class TestGetImport: def test_parses_completed_job(self) -> None: respx.get(f"{DEFAULT_BASE_URL}/v2/sample-imports/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, + "id": "42", "status": "COMPLETED", "created": 1700000000, "started": 1700000001, "finished": 1700000002, "accessions": ["ERR1160845", "ERR10677146"], - "sample_ids": [101, 102], - "execution_id": 7, + "sample_ids": ["101", "102"], + "execution_id": "7", "error": None, }), ) client = Client() - result = client.samples.get_import(SampleImportJobId(42)) + result = client.samples.get_import(SampleImportJobId("42")) assert result == SampleImportJob( - id=SampleImportJobId(42), + id=SampleImportJobId("42"), status="COMPLETED", created=1700000000, started=1700000001, finished=1700000002, accessions=["ERR1160845", "ERR10677146"], - sample_ids=[101, 102], - execution_id=7, + sample_ids=["101", "102"], + execution_id="7", error=None, ) @@ -1241,20 +1241,20 @@ def test_parses_completed_job(self) -> None: def test_parses_failed_job_with_error(self) -> None: respx.get(f"{DEFAULT_BASE_URL}/v2/sample-imports/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, + "id": "42", "status": "FAILED", "created": 1700000000, "started": 1700000001, "finished": 1700000002, "accessions": ["ERR1160845"], "sample_ids": [], - "execution_id": 7, + "execution_id": "7", "error": "download failed: connection reset", }), ) client = Client() - result = client.samples.get_import(SampleImportJobId(42)) + result = client.samples.get_import(SampleImportJobId("42")) assert result.status == "FAILED" assert result.error == "download failed: connection reset" @@ -1270,19 +1270,19 @@ def test_raises_not_found_for_unknown_job(self) -> None: client = Client() with pytest.raises(NotFoundError): - client.samples.get_import(SampleImportJobId(999)) + client.samples.get_import(SampleImportJobId("999")) @respx.mock def test_parses_job_with_only_id_and_status(self) -> None: respx.get(f"{DEFAULT_BASE_URL}/v2/sample-imports/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, + "id": "42", "status": "RUNNING", }), ) client = Client() - result = client.samples.get_import(SampleImportJobId(42)) + result = client.samples.get_import(SampleImportJobId("42")) assert result.created is None assert result.started is None @@ -1296,13 +1296,13 @@ def test_parses_job_with_only_id_and_status(self) -> None: def test_naive_timestamp_is_treated_as_utc(self) -> None: respx.get(f"{DEFAULT_BASE_URL}/v2/sample-imports/42").mock( return_value=httpx.Response(HTTPStatus.OK, json={ - "id": 42, + "id": "42", "status": "RUNNING", "started": "2024-04-05T19:34:38", }), ) client = Client() - result = client.samples.get_import(SampleImportJobId(42)) + result = client.samples.get_import(SampleImportJobId("42")) assert result.started == datetime(2024, 4, 5, 19, 34, 38, tzinfo=timezone.utc) From 46215b0b0eddb443f8686bbe47d2af7e78b41991 Mon Sep 17 00:00:00 2001 From: Martin Husbyn Date: Mon, 17 Aug 2026 19:50:40 +0100 Subject: [PATCH 2/4] fix(cli): --job-id rejects non-ASCII digits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- flowbio/cli/_samples.py | 2 +- tests/unit/cli/test_samples.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/flowbio/cli/_samples.py b/flowbio/cli/_samples.py index 159a365..7065e86 100644 --- a/flowbio/cli/_samples.py +++ b/flowbio/cli/_samples.py @@ -273,7 +273,7 @@ def _configure_import(import_parser: argparse.ArgumentParser) -> None: def _job_id(value: str) -> SampleImportJobId: # Ids are strings on the wire but always digits; a non-numeric one would # fail deep inside the server's query building rather than at the boundary. - if not value.isdigit(): + if not (value.isascii() and value.isdigit()): raise argparse.ArgumentTypeError(f"job id must be an integer, got {value!r}") return SampleImportJobId(value) diff --git a/tests/unit/cli/test_samples.py b/tests/unit/cli/test_samples.py index cfe8ef3..6ca5e55 100644 --- a/tests/unit/cli/test_samples.py +++ b/tests/unit/cli/test_samples.py @@ -1565,6 +1565,15 @@ def test_non_numeric_job_id_reports_clear_message(self, run_cli) -> None: assert "_job_id" not in result.stderr assert "job id" in result.stderr.lower() + def test_non_ascii_digit_job_id_reports_clear_message(self, run_cli) -> None: + result = run_cli( + "--token", TOKEN, "samples", "import-status", "--job-id", "٤٢", + ) + + assert result.exit_code == 2 + assert "_job_id" not in result.stderr + assert "job id" in result.stderr.lower() + @respx.mock def test_completed_job_with_no_sample_ids_reports_none(self, run_cli) -> None: respx.get(f"{SAMPLE_IMPORTS_URL}/42").mock( From 42b70aef694c145f49fac5699953ee2ecbade388 Mon Sep 17 00:00:00 2001 From: Martin Husbyn Date: Tue, 18 Aug 2026 12:35:57 +0100 Subject: [PATCH 3/4] fix(v2): accept integer ids from a server that hasn't shipped string ids yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- flowbio/v2/samples.py | 10 ++++++++-- tests/unit/v2/test_samples.py | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/flowbio/v2/samples.py b/flowbio/v2/samples.py index c7b3197..86ba32a 100644 --- a/flowbio/v2/samples.py +++ b/flowbio/v2/samples.py @@ -32,7 +32,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Literal, NewType -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator from flowbio.v2._pagination import PageIterator from flowbio.v2.exceptions import ( @@ -199,7 +199,7 @@ class SampleImportSpec: metadata: dict[str, str] | None = None -class SampleImportJob(BaseModel, frozen=True): +class SampleImportJob(BaseModel): """A batch job that imports one or more accessions into samples. All accessions submitted in one :meth:`SampleResource.import_samples` call @@ -208,6 +208,12 @@ class SampleImportJob(BaseModel, frozen=True): ``"COMPLETED"``. """ + # coerce_numbers_to_str: a server that hasn't shipped string ids yet + # (an on-prem install caught between upgrades) sends these as integers, + # and pydantic does not coerce int -> str in lax mode by default. Python + # ints are unbounded, so accepting one here loses no precision. + model_config = ConfigDict(frozen=True, coerce_numbers_to_str=True) + id: SampleImportJobId = Field(description="Unique identifier for this import job.") status: SampleImportStatus = Field(description="The job's current lifecycle state.") created: datetime | None = Field(default=None, description="When the job was created.") diff --git a/tests/unit/v2/test_samples.py b/tests/unit/v2/test_samples.py index 5324320..ae1857c 100644 --- a/tests/unit/v2/test_samples.py +++ b/tests/unit/v2/test_samples.py @@ -1272,6 +1272,33 @@ def test_raises_not_found_for_unknown_job(self) -> None: with pytest.raises(NotFoundError): client.samples.get_import(SampleImportJobId("999")) + @respx.mock + def test_parses_a_job_from_a_server_that_still_sends_integer_ids(self) -> None: + # An on-prem install between client and server upgrades is a normal + # state; pydantic does not coerce int -> str in lax mode by default, + # so a server that hasn't shipped string ids yet would otherwise + # raise ValidationError on every import/import-status call. + respx.get(f"{DEFAULT_BASE_URL}/v2/sample-imports/42").mock( + return_value=httpx.Response(HTTPStatus.OK, json={ + "id": 42, + "status": "COMPLETED", + "created": 1700000000, + "started": 1700000001, + "finished": 1700000002, + "accessions": ["ERR1160845"], + "sample_ids": [845739323725217744], + "execution_id": 7, + "error": None, + }), + ) + + client = Client() + result = client.samples.get_import(SampleImportJobId("42")) + + assert result.id == "42" + assert result.sample_ids == ["845739323725217744"] + assert result.execution_id == "7" + @respx.mock def test_parses_job_with_only_id_and_status(self) -> None: respx.get(f"{DEFAULT_BASE_URL}/v2/sample-imports/42").mock( From d7e452646bc97244990dc49ab8684b376b9b9f2c Mon Sep 17 00:00:00 2001 From: Martin Husbyn Date: Tue, 18 Aug 2026 12:36:00 +0100 Subject: [PATCH 4/4] refactor(cli): drop redundant str() over already-string sample ids job.sample_ids is list[str] now that the API serialises record ids as strings; str() on each element was a no-op. --- flowbio/cli/_samples.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flowbio/cli/_samples.py b/flowbio/cli/_samples.py index 7065e86..d82a759 100644 --- a/flowbio/cli/_samples.py +++ b/flowbio/cli/_samples.py @@ -667,7 +667,7 @@ def _import_status_command( def _job_summary(job: SampleImportJob) -> str: if job.status == "COMPLETED": - ids = ", ".join(str(sample_id) for sample_id in job.sample_ids) or "none" + ids = ", ".join(job.sample_ids) or "none" suffix = _timestamp_suffix("finished", job.finished) return f"Job {job.id}: COMPLETED{suffix}. Sample ids: {ids}." if job.status == "FAILED":