aglinxinyuan commented on code in PR #4206:
URL: https://github.com/apache/texera/pull/4206#discussion_r3354061624


##########
amber/src/main/python/core/models/operator.py:
##########
@@ -291,3 +291,30 @@ def process_table(self, table: Table, port: int) -> 
Iterator[Optional[TableLike]
             time, or None.
         """
         yield
+
+
+class LoopStartOperator(TableOperator):
+    @overrides.final
+    def process_state(self, state: State, port: int) -> Optional[State]:
+        if "LoopStartStateURI" in state:
+            state["loop_counter"] += 1
+            return state
+        self.state.update(state)
+        return None
+
+    @overrides.final
+    def produce_state_on_finish(self, port: int) -> State:
+        from pickle import dumps
+
+        self.state["table"] = 
dumps(Table(self._TableOperator__table_data[port]))
+        return dict(self.state)
+
+
+class LoopEndOperator(TableOperator):
+    @overrides.final
+    def process_table(self, table: Table, port: int) -> 
Iterator[Optional[TableLike]]:
+        yield table
+
+    @abstractmethod
+    def condition(self) -> bool:

Review Comment:
   Closed in 873bd33d87. Most of the substance had already landed; this commit 
adds the discoverability layer.
   
   | Concern | Status |
   |---|---|
   | Generator overrides `process_state` opaquely | **Fixed in 411d92f67** — 
the LoopStart/End generator templates collapsed to thin delegates. `LoopStart` 
does `yield self.eval_output($output, table)`; `LoopEnd` does 
`self.run_update($update, state)` and `return self.eval_condition($condition)`. 
All substantive logic lives in the Python base classes (`eval_output`, 
`run_update`, `eval_condition`). |
   | Reserved names as string conventions | **Mostly encoded** in prior commits 
— `loop_counter` / `LoopStartId` / `LoopStartStateURI` are typed fields on 
`StateFrame` (`core/models/payload.py`), not string keys in user state. `table` 
/ `output` were filtered out of `self.state` by hard-coded logic in each 
helper. |
   | Logic split across 3 files | **Generator already collapsed**; 
`LoopStartOpDescSpec` / `LoopEndOpDescSpec` pin that the emitted code uses only 
the base helpers (`code should include("self.eval_output(")` etc.) and contains 
no `loop_counter` logic. |
   | "Which method runs when" / "what `self.state` must contain after `open()` 
returns" — not encoded anywhere as code-level prose | **Closed here** — 
class-level docstrings on `LoopStartOperator` and `LoopEndOperator` now 
document the lifecycle, subclass contract, and reserved-name space inline with 
the code. Discoverable via `help(LoopStartOperator)`. |
   | Reserved-key set had no single discoverable source of truth | **Closed 
here** — new `_RESERVED_STATE_KEYS = frozenset({"table", "output"})` constant; 
the three filter sites in `eval_output` / `run_update` / 
`produce_state_on_finish` now read against this single source. 
`TestReservedStateKeysConstant` pins the set's contents (and that envelope-only 
names like `loop_counter` are NOT in it). |
   
   12/12 tests in `test_loop_operators.py` green (3 new + 9 existing). Diff 
scoped to `operator.py` (docstrings + constant + helper rewrites) and 
`test_loop_operators.py` (one new test class).



-- 
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]

Reply via email to