You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "carloea2 (via GitHub)" <gi...@apache.org> on 2026/08/31 08:02:12 UTC
[PR] fix(pyamber): honor output port lookup [texera]
carloea2 opened a new pull request, #8276:
URL: https://github.com/apache/texera/pull/8276
### What changes were proposed in this PR?
Honor an explicit port ID when retrieving an output port. Calls without an ID retain the existing first-port behavior, while unknown explicit IDs now fail with `KeyError`.
### Any related issues, documentation, discussions?
Closes #8274
### How was this PR tested?
Added positive coverage for a requested nonfirst port and negative coverage for an unknown port.
`C:\Users\carlo\texera\texera\.venv312\Scripts\python.exe -c "import sys,pytest; sys.path[:0]=[r'C:\Users\carlo\texera\texera-worktrees\investigate-bug73\amber\src\main\python',r'C:\Users\carlo\texera\texera\amber\src\main\python']; raise SystemExit(pytest.main([r'amber\src\test\python\core\architecture\packaging\test_output_manager.py','-p','no:cacheprovider','-q']))"`
`C:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe check amber/src/main/python/core/architecture/packaging/output_manager.py amber/src/test/python/core/architecture/packaging/test_output_manager.py`
`C:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe format --check amber/src/main/python/core/architecture/packaging/output_manager.py amber/src/test/python/core/architecture/packaging/test_output_manager.py`
All 41 tests passed. Ruff checks passed.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(pyamber): honor output port lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #8276:
URL: https://github.com/apache/texera/pull/8276#issuecomment-6303905770
/request-review @sshiv012 @parshimers
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(pyamber): honor output port lookup [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8276:
URL: https://github.com/apache/texera/pull/8276#issuecomment-5475555934
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/8276?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 93.93%. Comparing base ([`50321e4`](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`990e4b5`](https://app.codecov.io/gh/apache/texera/commit/990e4b5b13d3f2d078eac6262eed0ee51052e8bb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8276 +/- ##
=========================================
Coverage 93.93% 93.93%
Complexity 4761 4761
=========================================
Files 1191 1191
Lines 48308 48310 +2
Branches 5377 5377
=========================================
+ Hits 45378 45380 +2
Misses 1495 1495
Partials 1435 1435
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `99.32% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.02% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.67% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.86% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.91% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.41% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.31% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.87% <100.00%> (+<0.01%)` | :arrow_up: | |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/8276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.19% <ø> (ø)` | | Carriedforward from [50321e4](https://app.codecov.io/gh/apache/texera/commit/50321e403c82df299a13deb50a7f9849dd93bdba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/8276?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(pyamber): honor output port lookup [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #8276:
URL: https://github.com/apache/texera/pull/8276#discussion_r3910929114
##########
amber/src/main/python/core/architecture/packaging/output_manager.py:
##########
@@ -169,7 +169,9 @@ def start_writer(uri: str, name_prefix: str, registry: dict) -> None:
)
def get_port(self, port_id=None) -> WorkerPort:
- return list(self._ports.values())[0]
+ if port_id is None:
+ return next(iter(self._ports.values()))
Review Comment:
The empty-`_ports` failure mode changes shape here. `list(self._ports.values())[0]` raised `IndexError: list index out of range`. `next(iter(...))` raises a bare, message-less `StopIteration` — and through the emit generator expressions, PEP 479 turns that into `RuntimeError: generator raised StopIteration`.
Empty `_ports` is a documented legitimate state — `is_missing_output_ports` exists for it — so the branch is not dead by construction. Advisory rather than blocking because no production caller reaches `get_port()` in that state.
The narrow fix is to keep the old failure visible to callers — wrap the `next(...)` so an exhausted iterator surfaces as the `IndexError` this function used to raise, rather than as a bare `StopIteration` the generator machinery reinterprets.
##########
amber/src/test/python/core/architecture/packaging/test_output_manager.py:
##########
@@ -666,6 +666,18 @@ def test_get_port_ids_returns_added_ports_in_insertion_order(self, output_manage
output_manager.add_output_port(port_b, MagicMock())
assert output_manager.get_port_ids() == [port_a, port_b]
+ def test_get_port_honors_the_requested_port_id(self, output_manager):
+ port_a = PortIdentity(id=0, internal=False)
+ port_b = PortIdentity(id=1, internal=False)
+ output_manager.add_output_port(port_a, MagicMock(name="port_a"))
+ output_manager.add_output_port(port_b, MagicMock(name="port_b"))
+ assert output_manager.get_port(port_b) is output_manager._ports[port_b]
Review Comment:
This asserts against the private `_ports` dict, so it passes for any implementation that stores the port under that key — including one that returns the wrong schema. The file's own pattern at :397 asserts on the observable schema instead, which is what the caller actually depends on.
##########
amber/src/main/python/core/architecture/packaging/output_manager.py:
##########
@@ -169,7 +169,9 @@ def start_writer(uri: str, name_prefix: str, registry: dict) -> None:
)
def get_port(self, port_id=None) -> WorkerPort:
- return list(self._ports.values())[0]
+ if port_id is None:
+ return next(iter(self._ports.values()))
Review Comment:
`port_id` now carries a two-mode contract — an explicit `PortIdentity`, or the implicit single-port fallback — but it is neither annotated nor documented. `InputManager.get_port` and `add_output_port` in this same file both annotate theirs.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(pyamber): honor output port lookup [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8276:
URL: https://github.com/apache/texera/pull/8276#issuecomment-5475576662
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 2 better · 🔴 5 worse · ⚪ 8 noise (<±5%) · 0 without baseline
> Compared against main `50321e4` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/33371028573)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 453 | 0.276 | 21,941/29,961/29,961 us | 🔴 +21.4% / 🔴 +102.6% |
| 🟢 | bs=100 sw=10 sl=64 | 1,185 | 0.724 | 83,572/93,037/93,037 us | 🟢 -24.0% / 🟢 -14.4% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,394 | 0.851 | 711,785/809,701/809,701 us | ⚪ within ±5% / 🟢 +24.5% |
<details><summary>Baseline details</summary>
Latest main `50321e4` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 453 tuples/sec | 547 tuples/sec | 844.57 tuples/sec | -17.2% | -46.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.276 MB/s | 0.334 MB/s | 0.515 MB/s | -17.4% | -46.5% |
| bs=10 sw=10 sl=64 | p50 | 21,941 us | 18,423 us | 11,837 us | +19.1% | +85.4% |
| bs=10 sw=10 sl=64 | p95 | 29,961 us | 24,682 us | 14,788 us | +21.4% | +102.6% |
| bs=10 sw=10 sl=64 | p99 | 29,961 us | 24,682 us | 17,651 us | +21.4% | +69.7% |
| bs=100 sw=10 sl=64 | throughput | 1,185 tuples/sec | 1,175 tuples/sec | 1,084 tuples/sec | +0.9% | +9.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.724 MB/s | 0.717 MB/s | 0.662 MB/s | +1.0% | +9.4% |
| bs=100 sw=10 sl=64 | p50 | 83,572 us | 81,530 us | 94,505 us | +2.5% | -11.6% |
| bs=100 sw=10 sl=64 | p95 | 93,037 us | 122,380 us | 100,941 us | -24.0% | -7.8% |
| bs=100 sw=10 sl=64 | p99 | 93,037 us | 122,380 us | 108,642 us | -24.0% | -14.4% |
| bs=1000 sw=10 sl=64 | throughput | 1,394 tuples/sec | 1,415 tuples/sec | 1,119 tuples/sec | -1.5% | +24.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.851 MB/s | 0.864 MB/s | 0.683 MB/s | -1.5% | +24.5% |
| bs=1000 sw=10 sl=64 | p50 | 711,785 us | 709,041 us | 924,404 us | +0.4% | -23.0% |
| bs=1000 sw=10 sl=64 | p95 | 809,701 us | 818,883 us | 967,785 us | -1.1% | -16.3% |
| bs=1000 sw=10 sl=64 | p99 | 809,701 us | 818,883 us | 995,630 us | -1.1% | -18.7% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,441.55,200,128000,453,0.276,21940.84,29961.09,29961.09
1,100,10,64,20,1687.21,2000,1280000,1185,0.724,83572.29,93037.19,93037.19
2,1000,10,64,20,14351.99,20000,12800000,1394,0.851,711785.39,809700.97,809700.97
```
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(pyamber): honor output port lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #8276:
URL: https://github.com/apache/texera/pull/8276#issuecomment-5545679874
@Yicong-Huang All review comments are addressed and checks are green. Could you take another look?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(pyamber): honor output port lookup [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8276:
URL: https://github.com/apache/texera/pull/8276#issuecomment-5475508053
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@aglinxinyuan`, `@eugenegujing`
You can notify them by mentioning `@aglinxinyuan`, `@eugenegujing` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(pyamber): honor output port lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #8276:
URL: https://github.com/apache/texera/pull/8276#discussion_r3935988524
##########
amber/src/main/python/core/architecture/packaging/output_manager.py:
##########
@@ -169,7 +169,9 @@ def start_writer(uri: str, name_prefix: str, registry: dict) -> None:
)
def get_port(self, port_id=None) -> WorkerPort:
- return list(self._ports.values())[0]
+ if port_id is None:
+ return next(iter(self._ports.values()))
Review Comment:
Preserved the existing IndexError and added a regression test for the empty-port case.
##########
amber/src/test/python/core/architecture/packaging/test_output_manager.py:
##########
@@ -666,6 +666,18 @@ def test_get_port_ids_returns_added_ports_in_insertion_order(self, output_manage
output_manager.add_output_port(port_b, MagicMock())
assert output_manager.get_port_ids() == [port_a, port_b]
+ def test_get_port_honors_the_requested_port_id(self, output_manager):
+ port_a = PortIdentity(id=0, internal=False)
+ port_b = PortIdentity(id=1, internal=False)
+ output_manager.add_output_port(port_a, MagicMock(name="port_a"))
+ output_manager.add_output_port(port_b, MagicMock(name="port_b"))
+ assert output_manager.get_port(port_b) is output_manager._ports[port_b]
Review Comment:
Changed the test to assert the selected port schema through the public method.
##########
amber/src/main/python/core/architecture/packaging/output_manager.py:
##########
@@ -169,7 +169,9 @@ def start_writer(uri: str, name_prefix: str, registry: dict) -> None:
)
def get_port(self, port_id=None) -> WorkerPort:
- return list(self._ports.values())[0]
+ if port_id is None:
+ return next(iter(self._ports.values()))
Review Comment:
Added the optional PortIdentity annotation and a short contract docstring.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(pyamber): honor output port lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #8276:
URL: https://github.com/apache/texera/pull/8276#discussion_r3936634574
##########
amber/src/test/python/core/architecture/packaging/test_output_manager.py:
##########
@@ -666,6 +666,29 @@ def test_get_port_ids_returns_added_ports_in_insertion_order(self, output_manage
output_manager.add_output_port(port_b, MagicMock())
assert output_manager.get_port_ids() == [port_a, port_b]
+ def test_get_port_honors_the_requested_port_id(self, output_manager):
+ port_a = PortIdentity(id=0, internal=False)
+ port_b = PortIdentity(id=1, internal=False)
+ schema_a = MagicMock(name="schema_a")
+ schema_b = MagicMock(name="schema_b")
+ output_manager.add_output_port(port_a, schema_a)
+ output_manager.add_output_port(port_b, schema_b)
+ assert output_manager.get_port(port_b).get_schema() is schema_b
+
+ def test_get_port_without_an_id_uses_the_first_port(self, output_manager):
+ schema = MagicMock(name="schema")
+ output_manager.add_output_port(PortIdentity(id=0), schema)
+ assert output_manager.get_port().get_schema() is schema
Review Comment:
Registered two ports so the fallback now proves insertion-order selection.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(pyamber): honor output port lookup [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8276:
URL: https://github.com/apache/texera/pull/8276#issuecomment-5475507459
<!-- backport-auto-label-report -->
### Backport auto-label report
This `fix:` PR was checked against each actively-supported release branch. `release/*` labels drive the post-merge backport, so add or remove one to change where this fix lands.
| Release branch | Analysis |
| --- | --- |
| ⚠️ `release/v1.3` | Not labeled automatically — none of the files this PR modifies exist on this branch (`amber/src/main/python/core/architecture/packaging/output_manager.py`, `amber/src/test/python/core/architecture/packaging/test_output_manager.py`). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. **Please check and add `release/v1.3` by hand if this fix should be backported here.** |
| ✅ `release/v1.2` | Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7. |
_[Auto-label run](https://github.com/apache/texera/actions/runs/33371011655)._
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(pyamber): honor output port lookup [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #8276:
URL: https://github.com/apache/texera/pull/8276#discussion_r3936582335
##########
amber/src/test/python/core/architecture/packaging/test_output_manager.py:
##########
@@ -666,6 +666,29 @@ def test_get_port_ids_returns_added_ports_in_insertion_order(self, output_manage
output_manager.add_output_port(port_b, MagicMock())
assert output_manager.get_port_ids() == [port_a, port_b]
+ def test_get_port_honors_the_requested_port_id(self, output_manager):
+ port_a = PortIdentity(id=0, internal=False)
+ port_b = PortIdentity(id=1, internal=False)
+ schema_a = MagicMock(name="schema_a")
+ schema_b = MagicMock(name="schema_b")
+ output_manager.add_output_port(port_a, schema_a)
+ output_manager.add_output_port(port_b, schema_b)
+ assert output_manager.get_port(port_b).get_schema() is schema_b
+
+ def test_get_port_without_an_id_uses_the_first_port(self, output_manager):
+ schema = MagicMock(name="schema")
+ output_manager.add_output_port(PortIdentity(id=0), schema)
+ assert output_manager.get_port().get_schema() is schema
Review Comment:
With one port registered, "first" and "only" are the same thing — an implementation that returned the *last* registered port would pass this unchanged. The insertion-order choice is what `tuple_to_frame` relies on (`output_manager.py:361`, `:363`), so it is worth pinning. Registering a second port makes the selection observable, the way the case above does.
```suggestion
port_a = PortIdentity(id=0, internal=False)
port_b = PortIdentity(id=1, internal=False)
schema_a = MagicMock(name="schema_a")
output_manager.add_output_port(port_a, schema_a)
output_manager.add_output_port(port_b, MagicMock(name="schema_b"))
assert output_manager.get_port().get_schema() is schema_a
```
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(pyamber): honor output port lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #8276:
URL: https://github.com/apache/texera/pull/8276#issuecomment-6303870015
Could you review this small change when you have time?
/request-review @sshiv012 @parshimers
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org