Skip to content

Commit f4f1546

Browse files
authored
Merge branch 'main' into eywalker/itl-563-empty-failed-outputs-lose-tag-nullability-and-abort
2 parents 6b16ee7 + 42dc055 commit f4f1546

11 files changed

Lines changed: 1409 additions & 49 deletions

DESIGN_ISSUES.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,36 @@ which column groups (meta, source, system_tags) are returned.
384384

385385
---
386386

387+
### F15 — `FunctionPod` has no pod-level error policy; sync and async paths are inconsistent
388+
**Status:** open
389+
**Severity:** high
390+
**Issue:** ITL-527
391+
392+
`FunctionPod` has no `on_error` configuration. The two execution paths behave differently:
393+
394+
- **Sync path** (`FunctionPodStream._iter_data_*`): no exception handling at all — exceptions
395+
propagate unconditionally out of the iterator.
396+
- **Async path** (`_FunctionPodBase.async_execute()`): exceptions are caught, the observer is
397+
notified via `on_data_crash()`, and the item is **silently dropped from output** with no
398+
indication to the caller and no way to configure this behaviour.
399+
400+
The node-level `FunctionJobNode.execute()` adds `error_policy: Literal["continue", "fail_fast"]`,
401+
but this is only available for DB-backed execution and uses different vocabulary from the rest
402+
of the framework.
403+
404+
`SinkPod` and `TapPod` (ITL-524/ITL-525) introduced a clean `on_error: Literal["raise", "log"]`
405+
vocabulary at the pod level. `FunctionPod` should adopt the same model: pod-level `on_error`
406+
config, consistent behaviour between sync and async paths, and the same `"raise"` / `"log"`
407+
vocabulary.
408+
409+
**Fix needed:** Add `on_error: Literal["raise", "log"] = "raise"` to `FunctionPodConfig` (or
410+
equivalent). In the async path, replace the unconditional silent-drop with the configured
411+
behaviour: `"raise"` propagates the exception; `"log"` logs at `WARNING` and drops the item
412+
(current behaviour, but now explicit and configurable). Align `FunctionJobNode.error_policy`
413+
to use the same vocabulary.
414+
415+
---
416+
387417
## `src/orcapod/core/cached_function_pod.py` / `src/orcapod/core/data_function.py`
388418

389419
### CFP1 — Extract shared result caching logic from CachedDataFunction and CachedFunctionPod
@@ -1291,3 +1321,22 @@ across this version range.
12911321

12921322
**Ongoing:** pyspiral releases frequently. See PLT-1785 for the tracking issue
12931323
covering routine version bumps.
1324+
1325+
---
1326+
1327+
## `src/orcapod/core/nodes/operator_node.py`
1328+
1329+
### ON1 — OperatorJobNode has no v0→v1 schema migration for `NODE_CONTENT_HASH_COL`
1330+
**Status:** open
1331+
**Severity:** high
1332+
**Issue:** ITL-539
1333+
1334+
`OperatorJobNode` stores `NODE_CONTENT_HASH_COL` as `large_binary` (changed in ITL-539),
1335+
but unlike `FunctionJobNode` and `ResultCache`, it has no `_ensure_schema()` guard and no
1336+
v0→v1 migration utility. Any existing pipeline DB written by the old code that stored
1337+
`NODE_CONTENT_HASH_COL` as `large_string` will fail with an Arrow schema mismatch on the
1338+
next write attempt. This is accepted as a breaking change for a pre-v0.1.0 project; users
1339+
must drop and recreate the affected pipeline DB tables manually.
1340+
1341+
A proper migration utility (analogous to `migrate_pipeline_v0_to_v1`) should be implemented
1342+
before v0.1.0 ship.

src/orcapod/core/data_function.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,10 +463,10 @@ def __init__(
463463
semantic_hasher = self.data_context.semantic_hasher
464464
self._function_signature_hash = semantic_hasher.hash_object(
465465
get_function_signature(function)
466-
).to_string()
466+
).to_prefixed_digest()
467467
self._function_content_hash = semantic_hasher.hash_object(
468468
get_function_components(self._function)
469-
).to_string()
469+
).to_prefixed_digest()
470470

471471
@property
472472
def canonical_function_name(self) -> str:
@@ -486,8 +486,8 @@ def get_function_variation_data_schema(self) -> Schema:
486486
"""Schema for the data returned by ``get_function_variation_data``."""
487487
return Schema({
488488
"function_name": str,
489-
"function_signature_hash": str,
490-
"function_content_hash": str,
489+
"function_signature_hash": bytes,
490+
"function_content_hash": bytes,
491491
"git_hash": str,
492492
})
493493

src/orcapod/core/nodes/operator_node.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,16 @@ def _filter_by_content_hash(self, table: pa.Table) -> pa.Table:
776776
f"required column {col_name!r} is missing from the stored table. "
777777
"This may indicate records written by an older version of the code."
778778
)
779-
own_hash = self.content_hash().to_string()
779+
col_type = table.schema.field(col_name).type
780+
if col_type != pa.large_binary():
781+
raise ValueError(
782+
f"Cannot isolate records for table_scope='pipeline_hash': "
783+
f"column {col_name!r} has type {col_type!r}, expected large_binary. "
784+
"This table was written by an older version of the code that stored "
785+
"this column as large_string. Drop and recreate the affected pipeline "
786+
"DB tables (see DESIGN_ISSUES.md ON1)."
787+
)
788+
own_hash = self.content_hash().to_prefixed_digest()
780789
mask = pc.equal(table.column(col_name), own_hash)
781790
return table.filter(mask)
782791

@@ -802,7 +811,10 @@ def _store_output_stream(self, stream: StreamProtocol) -> None:
802811
n_rows = output_table.num_rows
803812
output_table = output_table.append_column(
804813
constants.NODE_CONTENT_HASH_COL,
805-
pa.repeat(self.content_hash().to_string(), n_rows).cast(pa.large_string()),
814+
pa.array(
815+
[self.content_hash().to_prefixed_digest()] * n_rows,
816+
type=pa.large_binary(),
817+
),
806818
)
807819

808820
# Per-row record hashes for dedup: hash(tag + data + system_tags + node_content_hash).

src/orcapod/core/result_cache.py

Lines changed: 6 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@
1010
import logging
1111
import uuid
1212
from datetime import datetime, timezone
13-
from typing import TYPE_CHECKING
13+
from typing import TYPE_CHECKING, Any
1414

1515
from orcapod.errors import SchemaVersionError
1616
from orcapod.protocols.core_protocols import DataProtocol
1717
from orcapod.protocols.database_protocols import ArrowDatabaseProtocol
1818
from orcapod.system_constants import constants, RESULT_DB_SCHEMA_VERSION
19-
from orcapod.types import ContentHash
2019
from orcapod.utils.lazy_module import LazyModule
2120

2221
if TYPE_CHECKING:
@@ -29,20 +28,6 @@
2928
logger = logging.getLogger(__name__)
3029

3130

32-
def _hash_val_to_binary(val: "str | bytes | memoryview | None") -> "bytes | None":
33-
"""Convert a ContentHash value to its prefixed binary digest.
34-
35-
Tolerates both ``str`` (v0 format, passed through ``ContentHash.from_string``)
36-
and ``bytes``/``memoryview`` (already binary v1 format, returned as-is).
37-
Returns ``None`` for ``None`` inputs.
38-
"""
39-
if val is None:
40-
return None
41-
if isinstance(val, (bytes, memoryview)):
42-
return bytes(val)
43-
return ContentHash.from_string(val).to_prefixed_digest()
44-
45-
4631
# Process-level cache of v1 result DB paths that have already been checked for
4732
# legacy v0 schema. Populated on first access; prevents repeated table_exists
4833
# calls for the same path within a single process.
@@ -169,7 +154,7 @@ def _ensure_rdb_schema(self) -> None:
169154
def lookup(
170155
self,
171156
input_data: DataProtocol,
172-
additional_constraints: dict[str, str] | None = None,
157+
additional_constraints: dict[str, Any] | None = None,
173158
) -> DataProtocol | None:
174159
"""Look up a cached output data for *input_data*.
175160
@@ -183,7 +168,9 @@ def lookup(
183168
input_data: The input data whose content hash is the
184169
primary lookup key.
185170
additional_constraints: Optional extra column-value pairs to
186-
include in the lookup query.
171+
include in the lookup query. Values may be ``bytes`` (for
172+
binary hash columns) or other scalar types (e.g. ``str``
173+
for ``function_name``).
187174
188175
Returns:
189176
The cached output data with ``RESULT_COMPUTED_FLAG: False``
@@ -195,7 +182,7 @@ def lookup(
195182

196183
RECORD_ID_COL = "_record_id"
197184

198-
constraints: dict[str, bytes] = {
185+
constraints: dict[str, Any] = {
199186
constants.INPUT_DATA_HASH_COL: input_data.content_hash().to_prefixed_digest(),
200187
}
201188
if additional_constraints:
@@ -282,22 +269,6 @@ def store(
282269
)
283270
col_idx += 1
284271

285-
# Convert ContentHash variation columns to large_binary (v1 schema).
286-
# Tolerates both str (v0 format) and bytes/memoryview (already binary — pass through).
287-
_HASH_VAR_COLS = {
288-
f"{constants.PF_VARIATION_PREFIX}function_signature_hash",
289-
f"{constants.PF_VARIATION_PREFIX}function_content_hash",
290-
}
291-
for col_name in _HASH_VAR_COLS:
292-
if col_name in data_table.column_names:
293-
col_idx = data_table.column_names.index(col_name)
294-
raw_vals = data_table.column(col_name).to_pylist()
295-
binary_vals = pa.array(
296-
[_hash_val_to_binary(v) for v in raw_vals],
297-
type=pa.large_binary(),
298-
)
299-
data_table = data_table.set_column(col_idx, col_name, binary_vals)
300-
301272
# Add input data hash as large_binary at position 0 (v1 schema).
302273
data_table = data_table.add_column(
303274
0,

src/orcapod/side_effects.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ def _execute_side_effect_row(
264264
pipeline_database=pipeline_database,
265265
table_path=table_path,
266266
record_id=record_id,
267-
record_id_hash_str=record_id_hash.to_string(),
267+
record_id_hash_bytes=record_id_hash.to_prefixed_digest(),
268268
run_id=run_id,
269269
)
270270
return (tag, data)
@@ -284,7 +284,7 @@ def _write_invocation_row(
284284
pipeline_database: ArrowDatabaseProtocol,
285285
table_path: tuple[str, ...],
286286
record_id: bytes,
287-
record_id_hash_str: str,
287+
record_id_hash_bytes: bytes,
288288
run_id: str | None,
289289
) -> None:
290290
"""Write one success row to the side-effect invocation log table.
@@ -298,15 +298,15 @@ def _write_invocation_row(
298298
table_path: Path tuple for the invocation log table.
299299
record_id: Deterministic bytes key for this ``(input, pod version)``
300300
pair — the prefixed digest of the unified preimage hash.
301-
record_id_hash_str: String form of the record-ID hash (stored for
302-
human inspection).
301+
record_id_hash_bytes: Binary prefixed digest of the record-ID hash
302+
(stored for human inspection via ``ContentHash.from_prefixed_digest``).
303303
run_id: Pipeline run identifier (or ``None`` for standalone mode).
304304
"""
305305
executed_at = datetime.datetime.now(datetime.timezone.utc)
306306
record = pa.table(
307307
{
308308
"record_id_hash": pa.array(
309-
[record_id_hash_str], type=pa.large_string()
309+
[record_id_hash_bytes], type=pa.large_binary()
310310
),
311311
"pipeline_run_id": pa.array(
312312
[run_id], type=pa.large_string()

0 commit comments

Comments
 (0)