Fix Source.socat_id type: int -> UUID, matching socat's real IDs - #12
Merged
Conversation
socat's source IDs have been UUIDs for as long as this repo's own
integrations/socat.py has existed (get_box_fixed() returns
RegisteredFixedSource.source_id: uuid.UUID, and upsert_sources() already
passes that straight through to backend.get_by_socat_id() and
Source(socat_id=...)) -- but Source.socat_id, SourceMetadata.socat_id,
the ProvidesSourceStorage.get_by_socat_id Protocol, the Postgres
sources.socat_id column (INTEGER UNIQUE), and the Postgres/parquet
storage implementations were all still typed/declared as int. Any real
socat crossmatch (a UUID) hitting this path would fail: Postgres would
reject the UUID value for an integer column, and the parquet backend's
`table["socat_id"] == socat_id` comparison would never match after an
int-typed column got created from UUID objects.
Fixed end to end: models/source.py, storage/prototype/source.py
(Protocol), storage/postgres/{schema,source}.py, and
storage/parquet/source.py (which additionally needed the same
str-round-trip treatment already given to source_id, since parquet has
no native UUID type -- with explicit None-handling, since socat_id is
optional and pandas represents a missing value in an otherwise-string
column as NaN, not None).
tests/test_integrations/test_socat.py::test_insert already exercised
this exact path with real UUIDs (via socat.client.mock.Client), but
only through the session-scoped `backend` fixture, which requires
Docker/testcontainers for every parametrization including "parquet"
(the shared `postgres_database`/`timescale_database` fixtures are
unconditional function arguments, and setup_test_data is autouse=True,
so no test in this suite can currently run without Docker). Verified
the fix with an equivalent standalone script directly against
generate_pandas_backend() (no Docker), confirming create -> upsert ->
get_by_socat_id round-trips correctly by UUID equality, and that a
second upsert_sources() call is idempotent. Also added
tests/test_integrations/test_socat_parquet.py, which exercises the
same path by building a Pandas backend directly rather than going
through the Docker-gated `backend` fixture -- it will run standalone
once Docker is available (e.g. in CI), even though this sandboxed
environment couldn't execute it directly (the autouse setup_test_data
fixture still pulls in the Docker-gated `backend` fixture for every
test in this directory regardless of what an individual test declares).
Co-Authored-By: Claude Sonnet 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
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.
socat's source IDs have been UUIDs for as long as this repo's own integrations/socat.py has existed (get_box_fixed() returns RegisteredFixedSource.source_id: uuid.UUID, and upsert_sources() already passes that straight through to backend.get_by_socat_id() and Source(socat_id=...)) -- but Source.socat_id, SourceMetadata.socat_id, the ProvidesSourceStorage.get_by_socat_id Protocol, the Postgres sources.socat_id column (INTEGER UNIQUE), and the Postgres/parquet storage implementations were all still typed/declared as int. Any real socat crossmatch (a UUID) hitting this path would fail: Postgres would reject the UUID value for an integer column, and the parquet backend's
table["socat_id"] == socat_idcomparison would never match after an int-typed column got created from UUID objects.Fixed end to end: models/source.py, storage/prototype/source.py (Protocol), storage/postgres/{schema,source}.py, and storage/parquet/source.py (which additionally needed the same str-round-trip treatment already given to source_id, since parquet has no native UUID type -- with explicit None-handling, since socat_id is optional and pandas represents a missing value in an otherwise-string column as NaN, not None).
tests/test_integrations/test_socat.py::test_insert already exercised this exact path with real UUIDs (via socat.client.mock.Client), but only through the session-scoped
backendfixture, which requires Docker/testcontainers for every parametrization including "parquet" (the sharedpostgres_database/timescale_databasefixtures are unconditional function arguments, and setup_test_data is autouse=True, so no test in this suite can currently run without Docker). Verified the fix with an equivalent standalone script directly against generate_pandas_backend() (no Docker), confirming create -> upsert -> get_by_socat_id round-trips correctly by UUID equality, and that a second upsert_sources() call is idempotent. Also added tests/test_integrations/test_socat_parquet.py, which exercises the same path by building a Pandas backend directly rather than going through the Docker-gatedbackendfixture -- it will run standalone once Docker is available (e.g. in CI), even though this sandboxed environment couldn't execute it directly (the autouse setup_test_data fixture still pulls in the Docker-gatedbackendfixture for every test in this directory regardless of what an individual test declares).