eugenegujing opened a new pull request, #7750:
URL: https://github.com/apache/texera/pull/7750
### What changes were proposed in this PR?
Adds `test_add_input_channel_handler.py` (12 tests) for
`core/architecture/handlers/control/add_input_channel_handler.py`. The
handler's three-line `is_control` block looks like a no-op but is load-bearing:
`Message.__hash__` is patched to hash-of-repr, betterproto's repr omits fields
left at their default, and proto3 never puts `false` on the wire — so a data
channel's `ChannelIdentity` arrives `==`-equal to its canonical form yet
hashing differently, and `InputManager` keys its channel registry by that hash.
`test_main_loop.py` drives AddInputChannel end to end on the happy path but
never pins the materialization, so the workaround could regress silently.
Every request in the spec is built through a bytes round trip so the handler
receives the real wire shape — an in-process request would arrive already
canonical and prove nothing. The spec pins:
- the delegation: `register_input` is the input manager's only call, with
the request's channel and port, and the handler returns `EmptyReturn`;
- the channel id is canonical by the time `register_input` receives it,
asserted by snapshotting the hash inside a side effect — a key inserted at the
pre-materialization hash and then mutated in place strands the dict entry;
- the consequences on a real `InputManager`: the registered channel resolves
through `get_port_id`, survives the `is_control` read that
`get_all_data_channel_ids()` performs on every key, and is interchangeable as a
dict key with an independently built canonical id in both directions;
- control channels arrive already canonical (proto3 does serialize `true`)
and keep their marker, guarding the `if not` condition against inversion;
- port 0, the default port, registers correctly even though its identity
arrives as a bare `PortIdentity()` (proto3 omits both `id=0` and
`internal=False`). This is complementary to
`test_input_manager.py::TestPortIdentityDefaults`, which feeds a hand-built
`PortIdentity(id=None, internal=None)` — a shape no production caller of
`register_input` produces; the wire shape exercised here is what the two
wire-fed callers actually deliver, and it is canonicalized by attribute reads
alone, not by the `is None` branches;
- a canary pins the wire premise itself, so a betterproto upgrade that
starts materializing defaults on parse reports itself instead of quietly making
the rest of the file meaningless.
### Any related issues, documentation, discussions?
Closes #7748.
### How was this PR tested?
12 new tests, all green locally:
```
cd amber
pytest
src/test/python/core/architecture/handlers/control/test_add_input_channel_handler.py
-q # 12 passed
pytest src/test/python/core/architecture/ -q
# 303 passed
pytest src/test/python/core/runnables/test_main_loop.py -q
# 34 passed
pytest -m "not integration" -q
# 1068 passed
ruff check src/main/python src/test/python && ruff format --check
src/main/python src/test/python
```
The only full-suite failures are the pre-existing
`core/storage/iceberg/test_iceberg_document.py` items that need a local catalog
stack; they fail identically without this PR.
The tests were also mutation-checked: deleting the handler's `is_control`
block, always assigning `False` (guard inversion), canonicalizing after
`register_input` instead of before, swapping the delegation arguments, dropping
the call entirely, and deleting `register_input`'s `id is None` guard each turn
the expected tests red.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
