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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
