From 74f63f0d9af76fb5455621050b8e16188d6c7392 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Mon, 13 Jul 2026 20:54:44 -0700 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20project=20Iceberg=20user=5Fmeta=20ra?= =?UTF-8?q?w=20=E2=80=94=20VARCHAR=E2=86=92JSON=20cast=20double-encodes=20?= =?UTF-8?q?(#399)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit json_format(CAST(m.metadata AS JSON)) wraps the already-JSON varchar as a JSON string value instead of parsing it, so every Iceberg-path row warned "Athena metadata JSON was not an object" and degraded result metadata to the {key: value} fallback. Select the column raw like the parquet _packages-view path, and decode a double-encoded string defensively in _parse_user_meta. Co-Authored-By: Claude Fable 5 --- docker/src/package_query.py | 16 ++++++++- docker/tests/test_package_query.py | 54 ++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/docker/src/package_query.py b/docker/src/package_query.py index 488cc000..a089e271 100644 --- a/docker/src/package_query.py +++ b/docker/src/package_query.py @@ -232,6 +232,13 @@ def _parse_user_meta(self, raw_meta: Optional[str], *, pkg_name: str, key: str, try: parsed = json.loads(raw_meta) + if isinstance(parsed, str): + # Double-encoded (e.g. a JSON string wrapping the document, as + # produced by Trino's VARCHAR→JSON cast, #399): decode once more. + try: + parsed = json.loads(parsed) + except json.JSONDecodeError: + pass # genuinely a string value; falls to the warning below if isinstance(parsed, dict): return parsed self.logger.warning( @@ -490,6 +497,13 @@ def _build_iceberg_union_query(self, buckets: List[str], key: str, value: str) - ``TYPE_MISMATCH: Expression m.metadata is not of type ROW``, so we use json_extract_scalar just like the parquet _packages-view path. + The projection selects the column raw (``m.metadata AS user_meta``), + also like the parquet path. Do NOT wrap it in + ``json_format(CAST(m.metadata AS JSON))``: Trino's VARCHAR→JSON cast + does not parse the string — it wraps it as a JSON *string value* — + so json_format double-encodes and json.loads yields a str, not a + dict (#399). + Args: buckets: List of bucket names to search key: Metadata key to filter on (JSON path field name) @@ -507,7 +521,7 @@ def _build_iceberg_union_query(self, buckets: List[str], key: str, value: str) - r.pkg_name, r.timestamp, m.message, - json_format(CAST(m.metadata AS JSON)) AS user_meta, + m.metadata AS user_meta, '{b}' AS _src_bucket FROM "{idb}"."{b}_package_revision" r JOIN "{idb}"."{b}_package_manifest" m ON r.top_hash = m.top_hash diff --git a/docker/tests/test_package_query.py b/docker/tests/test_package_query.py index 640215ff..28e99eb9 100644 --- a/docker/tests/test_package_query.py +++ b/docker/tests/test_package_query.py @@ -1,5 +1,6 @@ """Tests for package_query module.""" +import json from unittest.mock import Mock, patch import pytest @@ -417,8 +418,11 @@ def test_iceberg_build_union_query(self, mock_role_manager_class): # Should reference both buckets assert "bucket-a" in sql assert "bucket-b" in sql - # Should serialize metadata deterministically for Python parsing - assert "json_format(CAST(m.metadata AS JSON)) AS user_meta" in sql + # metadata is already a JSON document string; project it raw. Wrapping + # it in json_format(CAST(... AS JSON)) double-encodes, because Trino's + # VARCHAR→JSON cast wraps the string instead of parsing it (#399). + assert "m.metadata AS user_meta" in sql + assert "json_format" not in sql # metadata is a JSON string (from user_meta), not a native STRUCT, so # it must be filtered via json_extract_scalar to avoid TYPE_MISMATCH. assert "json_extract_scalar(m.metadata, '$.experiment_id')" in sql @@ -506,6 +510,52 @@ def test_iceberg_non_object_metadata_keeps_package_match(self, mock_role_manager "experiment_id": "EXP-1", } + @patch("src.package_query.RoleManager") + def test_iceberg_double_encoded_metadata_parses(self, mock_role_manager_class): + """Double-encoded user_meta (a JSON string wrapping the document, as + produced by Trino's VARCHAR→JSON cast, #399) must round-trip to the + full metadata dict, not the {key: value} fallback.""" + mock_athena = Mock() + mock_glue = Mock() + + mock_role_manager = Mock() + mock_session = Mock() + mock_session.client.side_effect = lambda service, **kw: { + "athena": mock_athena, + "glue": mock_glue, + }[service] + mock_role_manager._get_or_create_session.return_value = (mock_session, None) + mock_role_manager.role_arn = None + mock_role_manager._session = None + mock_role_manager._expires_at = None + mock_role_manager_class.return_value = mock_role_manager + + query = PackageQuery( + bucket="", + catalog_url="catalog.example.com", + database="test_db", + region="us-west-2", + iceberg_database="iceberg_db", + ) + + metadata = {"experiment_id": "EXP-1", "entry_id": "etr_123", "canvas_id": "cnvs_abc"} + query._list_iceberg_manifest_buckets = Mock(return_value=["bucket-a"]) + query._execute_query = Mock( + return_value=[ + { + "pkg_name": "benchling/pkg-a", + "timestamp": "latest", + "message": "A", + "user_meta": json.dumps(json.dumps(metadata)), + "_src_bucket": "bucket-a", + } + ] + ) + + result = query.find_unique_packages("experiment_id", "EXP-1") + + assert result["results"]["package_info"]["bucket-a/benchling/pkg-a"]["metadata"] == metadata + @patch("src.package_query.RoleManager") def test_iceberg_glue_access_denied_reports_glue_database(self, mock_role_manager_class): """Glue table-list permission failures should not be reported as Athena DB failures.""" From 325a3621c4fc7987338578c1a75f882b1d6d2fd3 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Mon, 13 Jul 2026 20:54:47 -0700 Subject: [PATCH 2/3] chore: bump version to 0.19.1 --- docker/app-manifest.yaml | 2 +- docker/pyproject.toml | 2 +- docker/uv.lock | 2 +- package.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docker/app-manifest.yaml b/docker/app-manifest.yaml index aed03bdc..c32dd5d2 100644 --- a/docker/app-manifest.yaml +++ b/docker/app-manifest.yaml @@ -2,7 +2,7 @@ manifestVersion: 1 info: name: nightly-quilttest-com description: Packaging Benchling Notebooks as Quilt packages - version: 0.19.0 + version: 0.19.1 features: - name: Quilt Connector id: quilt_entry diff --git a/docker/pyproject.toml b/docker/pyproject.toml index b50092dd..1428935f 100644 --- a/docker/pyproject.toml +++ b/docker/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "benchling-quilt-integration" -version = "0.19.0" +version = "0.19.1" description = "Benchling-Quilt Integration Webhook Service" license = {text = "Apache-2.0"} authors = [ diff --git a/docker/uv.lock b/docker/uv.lock index 66e33575..95abb7e5 100644 --- a/docker/uv.lock +++ b/docker/uv.lock @@ -75,7 +75,7 @@ wheels = [ [[package]] name = "benchling-quilt-integration" -version = "0.19.0" +version = "0.19.1" source = { editable = "." } dependencies = [ { name = "benchling-sdk" }, diff --git a/package.json b/package.json index 117338f2..559a566f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@quiltdata/benchling-webhook", - "version": "0.19.0", + "version": "0.19.1", "description": "AWS CDK deployment for Benchling webhook processing using Fargate - Deploy directly with npx", "main": "dist/lib/index.js", "types": "dist/lib/index.d.ts", From 6759e97c6d8d0e51242d22cd072fdc44c9b6ca41 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" Date: Mon, 13 Jul 2026 20:55:25 -0700 Subject: [PATCH 3/3] docs: changelog for 0.19.1 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a27db56..21008177 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +## [0.19.1] - 2026-07-14 + +### Fixed + +- Bucketless Iceberg lookup no longer double-encodes `user_meta`. The query projected `json_format(CAST(m.metadata AS JSON))`, but Trino's VARCHAR→JSON cast wraps the JSON document string as a JSON *string value* instead of parsing it — so every returned row logged "Athena metadata JSON was not an object" and the result's metadata silently degraded to the matched `{key: value}` fallback (canvas rendering was unaffected). The projection now selects the column raw, matching the legacy `_packages-view` path, and `_parse_user_meta` defensively decodes a double-encoded string (#399) + ## [0.19.0] - 2026-07-03 ### Added