Skip to content

Commit 7da1aa6

Browse files
fix(cli): make the run identity on the job-api DTOs explicit and pin the pooled resume version
JobKey has no default any more (the peer types it Guid; an empty string would be rejected), and resume_version is a required parameter on the sink installer and the connection helpers, so a lane states None on purpose rather than by omission. is_wire_job_id is is_wire_job_key. The pooled real-pipe test now sends ResumeVersion and asserts it on the received log.
1 parent 7087f5b commit 7da1aa6

6 files changed

Lines changed: 54 additions & 46 deletions

File tree

‎packages/uipath/src/uipath/_cli/_job_api.py‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class ExecutorJobStatus(IntEnum):
5858
class PythonJobLogDto:
5959
"""A log entry; field names are the wire keys (do not rename)."""
6060

61-
JobKey: str = ""
61+
JobKey: str
6262
ResumeVersion: int | None = None
6363
Message: str = ""
6464
LogLevel: int = LogLevel.INFORMATION.value
@@ -79,7 +79,7 @@ class JobExecutorError:
7979
class PythonJobResultDto:
8080
"""The final result; field names are the wire keys (do not rename)."""
8181

82-
JobKey: str = ""
82+
JobKey: str
8383
ResumeVersion: int | None = None
8484
Status: int = ExecutorJobStatus.SUCCESSFUL.value
8585
OutputArguments: Any = None
@@ -175,9 +175,9 @@ class _IpcLogHandler(logging.Handler):
175175
def __init__(
176176
self,
177177
job_key: str,
178+
resume_version: int | None,
178179
callback: Any,
179180
loop: asyncio.AbstractEventLoop,
180-
resume_version: int | None = None,
181181
) -> None:
182182
super().__init__()
183183
self._job_key = job_key
@@ -246,11 +246,14 @@ async def aflush_pending(self, timeout: float = _LOG_FLUSH_TIMEOUT_S) -> None:
246246

247247
def install_runtime_sinks(
248248
job_key: str,
249+
resume_version: int | None,
249250
callback: Any,
250251
loop: asyncio.AbstractEventLoop,
251-
resume_version: int | None = None,
252252
) -> "_IpcLogHandler | None":
253-
"""Install the log + result sinks, forwarding to ``callback`` on ``loop``.
253+
"""Install the log + result sinks for the run ``(job_key, resume_version)``, forwarding to ``callback`` on ``loop``.
254+
255+
The peer routes a pooled callback by that pair exactly, so ``resume_version`` is the caller's
256+
decision: the value it was handed, or ``None`` on a lane that has none.
254257
255258
``loop`` must run on a different thread than the one the sinks are invoked on, or the result ack
256259
deadlocks. Raises if this runtime has no sinks to install into.
@@ -268,7 +271,7 @@ def install_runtime_sinks(
268271
"Install uipath-runtime>=0.13.5."
269272
) from e
270273

271-
handler = _IpcLogHandler(job_key, callback, loop, resume_version)
274+
handler = _IpcLogHandler(job_key, resume_version, callback, loop)
272275
handler.setFormatter(logging.Formatter("%(message)s"))
273276

274277
def _result_sink(result: Any, output_arguments_file_path: str) -> None:
@@ -351,21 +354,21 @@ def _shutdown(self) -> None:
351354
_stop_loop_thread(self._loop, self._thread, _SET_RESULT_TIMEOUT_S)
352355

353356

354-
def is_wire_job_id(job_id: str | None) -> TypeGuard[str]:
357+
def is_wire_job_key(job_key: str | None) -> TypeGuard[str]:
355358
"""The peer types the job key as a Guid, and routes nothing for one it can't match."""
356359
try:
357-
parsed = uuid.UUID(str(job_id))
360+
parsed = uuid.UUID(str(job_key))
358361
except ValueError:
359362
return False
360363
# All-zeros is Guid's default, so it is the one unusable value a caller reaches by omission.
361364
return parsed.int != 0
362365

363366

364367
def connect_handler_ipc(
365-
pipe: str, job_key: str | None, resume_version: int | None = None
368+
pipe: str, job_key: str | None, resume_version: int | None
366369
) -> _HandlerIpcConnection:
367370
"""Dial ``pipe`` on a dedicated loop/thread and install the sinks (see ``_HandlerIpcConnection``)."""
368-
if not is_wire_job_id(job_key):
371+
if not is_wire_job_key(job_key):
369372
raise RuntimeError(
370373
f"--handler-ipc-pipe needs UIPATH_JOB_KEY to be a job key; got {job_key!r}."
371374
)
@@ -404,7 +407,7 @@ async def _build() -> Any:
404407
_stop_loop_thread(loop, thread, _SET_RESULT_TIMEOUT_S)
405408
raise
406409
# Install in the caller's context: the sinks are contextvars, read in the job's own context at teardown.
407-
handler = install_runtime_sinks(job_key, proxy, loop, resume_version)
410+
handler = install_runtime_sinks(job_key, resume_version, proxy, loop)
408411
return _HandlerIpcConnection(client, loop, thread, handler)
409412

410413

@@ -416,7 +419,7 @@ async def disconnect_handler_ipc(conn: _HandlerIpcConnection) -> None:
416419

417420
@contextlib.asynccontextmanager
418421
async def handler_ipc_connection(
419-
pipe: str | None, job_key: str | None, resume_version: int | None = None
422+
pipe: str | None, job_key: str | None, resume_version: int | None
420423
) -> AsyncIterator[Any]:
421424
"""Connect (if ``pipe`` is set) and always disconnect on exit; yields the connection or None."""
422425
conn = (

‎packages/uipath/src/uipath/_cli/cli_run.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,8 @@ async def execute() -> None:
231231
from ._job_api import handler_ipc_connection
232232

233233
async with (
234-
handler_ipc_connection(handler_ipc_pipe, ctx.job_id),
234+
# No resume version reaches this lane; the per-job pipe already names the run.
235+
handler_ipc_connection(handler_ipc_pipe, ctx.job_id, None),
235236
ResourceOverwritesContext(
236237
lambda: read_resource_overwrites_from_file(ctx.runtime_dir)
237238
),

‎packages/uipath/src/uipath/_cli/cli_server_ipc.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,13 @@ async def RunJob(
109109
IPythonJobApi,
110110
clear_runtime_sinks,
111111
install_runtime_sinks,
112-
is_wire_job_id,
112+
is_wire_job_key,
113113
)
114114

115-
if not is_wire_job_id(request.JobKey):
115+
if not is_wire_job_key(request.JobKey):
116116
return PythonServerRunJobResult(
117117
ExitCode=1,
118-
Error=f"StreamOutputOverIpc needs a 'JobKey' that is a job id; got {request.JobKey!r}",
118+
Error=f"StreamOutputOverIpc needs a 'JobKey' that is a job key (Guid); got {request.JobKey!r}",
119119
)
120120

121121
if message is None or message.client is None:
@@ -134,7 +134,7 @@ async def RunJob(
134134

135135
def _install() -> None:
136136
installed.append(
137-
install_runtime_sinks(job_key, callback, loop, resume_version)
137+
install_runtime_sinks(job_key, resume_version, callback, loop)
138138
)
139139

140140
on_run_start = _install

‎packages/uipath/tests/cli/test_job_api.py‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def test_dto_wire_key_sets_are_pinned():
116116
"Info",
117117
"Error",
118118
}
119-
assert set(to_wire(_job_api.PythonJobLogDto(Message="m"))) == {
119+
assert set(to_wire(_job_api.PythonJobLogDto(JobKey="j", Message="m"))) == {
120120
"JobKey",
121121
"ResumeVersion",
122122
"Message",
@@ -174,7 +174,7 @@ async def SetResult(self, dto: Any) -> bool:
174174

175175
async def scenario() -> None:
176176
loop = asyncio.get_running_loop()
177-
_job_api.install_runtime_sinks("job-7", _Callback(), loop, resume_version=2)
177+
_job_api.install_runtime_sinks("job-7", 2, _Callback(), loop)
178178

179179
# The log handler forwards each record, tagged with the run it belongs to.
180180
handler = captured["handler"]
@@ -209,7 +209,7 @@ async def SendLog(self, log: Any) -> None:
209209

210210
loop = asyncio.new_event_loop()
211211
try:
212-
_job_api.install_runtime_sinks(JOB_ID, _Callback(), loop)
212+
_job_api.install_runtime_sinks(JOB_ID, None, _Callback(), loop)
213213
handler = captured["handler"]
214214
handled: list[Any] = []
215215
monkeypatch.setattr(handler, "handleError", handled.append)
@@ -238,7 +238,7 @@ async def SendLog(self, dto: Any) -> None:
238238
thread = threading.Thread(target=loop.run_forever, daemon=True)
239239
thread.start()
240240
try:
241-
_job_api.install_runtime_sinks("job-6", _Callback(), loop)
241+
_job_api.install_runtime_sinks("job-6", None, _Callback(), loop)
242242
handler = captured["handler"]
243243
for name in ("uipath_ipc.client.connection", _job_api.__name__):
244244
handler.emit(
@@ -286,7 +286,7 @@ def emit(self, record: logging.LogRecord) -> None:
286286
thread = threading.Thread(target=loop.run_forever, daemon=True)
287287
thread.start()
288288
try:
289-
_job_api.install_runtime_sinks("job-8", _Callback(), loop)
289+
_job_api.install_runtime_sinks("job-8", None, _Callback(), loop)
290290
captured["sink"](_Result(), "out.args")
291291
finally:
292292
loop.call_soon_threadsafe(loop.stop)
@@ -340,7 +340,7 @@ async def aclose(self) -> None:
340340
loop = asyncio.new_event_loop()
341341
thread = threading.Thread(target=loop.run_forever, daemon=True)
342342
thread.start()
343-
handler = _job_api.install_runtime_sinks("job-7", _Callback(), loop)
343+
handler = _job_api.install_runtime_sinks("job-7", None, _Callback(), loop)
344344
assert handler is not None
345345
handler.emit(logging.LogRecord("j", logging.INFO, "p", 1, "tail line", (), None))
346346

@@ -380,7 +380,7 @@ def emit(self, record: logging.LogRecord) -> None:
380380
thread = threading.Thread(target=loop.run_forever, daemon=True)
381381
thread.start()
382382
try:
383-
_job_api.install_runtime_sinks("job-9", _Callback(), loop)
383+
_job_api.install_runtime_sinks("job-9", None, _Callback(), loop)
384384
sink = captured["sink"]
385385
sink(_Result(), "out.args") # must not raise
386386
finally:
@@ -419,7 +419,7 @@ async def SetResult(self, result: Any) -> bool:
419419
async def scenario() -> None:
420420
# The context manager, not the two halves: cli_run.py only ever uses this, and a
421421
# wiring mistake inside it would leave every assertion below untouched.
422-
async with _job_api.handler_ipc_connection(pipe, JOB_ID):
422+
async with _job_api.handler_ipc_connection(pipe, JOB_ID, None):
423423
assert captured["handler"] is not None
424424
assert captured["sink"] is not None
425425

@@ -447,7 +447,7 @@ def live() -> int:
447447
before = live()
448448

449449
with pytest.raises(RuntimeError, match="Could not reach the handler IPC pipe"):
450-
_job_api.connect_handler_ipc("uipath-jobapi-does-not-exist-12345", JOB_ID)
450+
_job_api.connect_handler_ipc("uipath-jobapi-does-not-exist-12345", JOB_ID, None)
451451

452452
assert live() == before
453453

@@ -466,7 +466,7 @@ async def SendLog(self, log: Any) -> None:
466466
thread = threading.Thread(target=loop.run_forever, daemon=True)
467467
thread.start()
468468
try:
469-
handler = _job_api.install_runtime_sinks(JOB_ID, _Callback(), loop)
469+
handler = _job_api.install_runtime_sinks(JOB_ID, None, _Callback(), loop)
470470
assert handler is not None
471471
for i in range(4):
472472
handler.emit(
@@ -492,7 +492,7 @@ def test_a_cancelled_send_is_reported_like_any_other_failure(monkeypatch):
492492

493493
loop = asyncio.new_event_loop()
494494
try:
495-
handler = _job_api._IpcLogHandler(JOB_ID, object(), loop)
495+
handler = _job_api._IpcLogHandler(JOB_ID, None, object(), loop)
496496
cancelled: Future[object] = Future()
497497
assert cancelled.cancel()
498498

@@ -508,7 +508,7 @@ def test_missing_output_sinks_fails_loudly(monkeypatch):
508508
monkeypatch.setitem(sys.modules, "uipath.runtime.output_sinks", None)
509509

510510
with pytest.raises(RuntimeError, match="uipath-runtime"):
511-
_job_api.install_runtime_sinks(JOB_ID, object(), asyncio.new_event_loop())
511+
_job_api.install_runtime_sinks(JOB_ID, None, object(), asyncio.new_event_loop())
512512

513513

514514
def test_the_frame_cap_matches_the_dotnet_peer():
@@ -544,14 +544,14 @@ def _live_ipc_threads() -> int:
544544
def test_connect_without_a_real_job_id_fails_fast(job_id):
545545
before = _live_ipc_threads()
546546
with pytest.raises(RuntimeError, match="UIPATH_JOB_KEY"):
547-
_job_api.connect_handler_ipc("pipe", job_id)
547+
_job_api.connect_handler_ipc("pipe", job_id, None)
548548
assert _live_ipc_threads() == before
549549

550550

551551
@pytest.mark.parametrize("job_id", [None, "", "job-1"])
552552
async def test_handler_ipc_connection_without_a_real_job_id_fails_fast(job_id):
553553
with pytest.raises(RuntimeError, match="UIPATH_JOB_KEY"):
554-
async with _job_api.handler_ipc_connection("pipe", job_id):
554+
async with _job_api.handler_ipc_connection("pipe", job_id, None):
555555
pass
556556

557557

@@ -647,7 +647,7 @@ class _Result:
647647
try:
648648

649649
async def scenario() -> None:
650-
conn = _job_api.connect_handler_ipc(pipe, JOB_ID_2, resume_version=1)
650+
conn = _job_api.connect_handler_ipc(pipe, JOB_ID_2, 1)
651651
# A log line has to survive the round trip too, not just the result.
652652
captured["handler"].emit(
653653
logging.LogRecord(

‎packages/uipath/tests/cli/test_run.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def test_the_job_runs_inside_the_open_connection(
235235
monkeypatch.setenv("UIPATH_JOB_KEY", job_key)
236236

237237
events: list[str] = []
238-
seen: list[tuple[str, str]] = []
238+
seen: list[tuple[str, str, int | None]] = []
239239
snapshots: list[object] = []
240240

241241
from uipath.runtime import context as runtime_context
@@ -244,8 +244,8 @@ def test_the_job_runs_inside_the_open_connection(
244244
sentinel = logging.Handler()
245245

246246
@asynccontextmanager
247-
async def _fake_connection(pipe, job_id):
248-
seen.append((pipe, job_id))
247+
async def _fake_connection(pipe, job_key, resume_version):
248+
seen.append((pipe, job_key, resume_version))
249249
events.append("open")
250250
# Install for real: the ordering that matters is against the runtime context's
251251
# one-shot snapshot, not against runtime.execute.
@@ -303,7 +303,7 @@ async def _execute(*a, **k):
303303
# The whole point: the connection must still be open when the job runs.
304304
assert events == ["open", "run", "close"], events
305305
# And it must be told which job, not None.
306-
assert seen == [("some-pipe", job_key)]
306+
assert seen == [("some-pipe", job_key, None)]
307307
# The connection must be open BEFORE the runtime context snapshots the sinks: move it
308308
# inside and __enter__ would capture None, losing every log line and the result.
309309
assert snapshots == [sentinel], (

‎packages/uipath/tests/cli/test_server_ipc.py‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ def test_runjob_without_a_real_job_key_fails_loudly(self, monkeypatch, job_key):
393393
monkeypatch.setattr(
394394
_job_api,
395395
"install_runtime_sinks",
396-
lambda jid, cb, loop, rv=None: events.append(("install", jid)),
396+
lambda jid, rv, cb, loop: events.append(("install", jid)),
397397
)
398398

399399
async def _fake_run(*args: Any, **kwargs: Any) -> dict[str, Any]:
@@ -450,14 +450,16 @@ def test_pooled_streaming_works_over_a_real_pipe(self, monkeypatch):
450450
"""No hand-fed ``message``: the dispatcher must inject it from the contract."""
451451
from uipath._cli import _job_api, cli_server_ipc
452452

453-
installed: list[Any] = []
453+
installed: list[tuple[str, int | None]] = []
454454
real_install = _job_api.install_runtime_sinks
455455

456-
def _record(jid, cb, loop, rv=None):
457-
installed.append(jid)
456+
def _record(
457+
jid: str, rv: int | None, cb: Any, loop: asyncio.AbstractEventLoop
458+
) -> Any:
459+
installed.append((jid, rv))
458460
# Install for real: patching this away would leave the callback unused, so nothing
459461
# would notice if RunJob asked the peer for the wrong contract.
460-
return real_install(jid, cb, loop, rv)
462+
return real_install(jid, rv, cb, loop)
461463

462464
monkeypatch.setattr(_job_api, "install_runtime_sinks", _record)
463465

@@ -505,6 +507,7 @@ async def scenario() -> Any:
505507
Any,
506508
{
507509
"JobKey": JOB_ID,
510+
"ResumeVersion": 2,
508511
"Command": "run",
509512
"Args": [],
510513
"StreamOutputOverIpc": True,
@@ -517,11 +520,12 @@ async def scenario() -> Any:
517520
result = asyncio.run(scenario())
518521
assert result.Error is None, result.Error
519522
assert result.ExitCode == 0
520-
assert installed == [JOB_ID]
523+
assert installed == [(JOB_ID, 2)]
521524
# The contract passed to get_callback IS the endpoint key on the wire: ask for the wrong
522525
# one and every send is addressed to something the peer does not host.
523526
assert len(delivered) == 1, "no log line crossed the pooled callback"
524-
assert delivered[0][0] == JOB_ID
527+
# The peer routes by (JobKey, ResumeVersion) exactly; a null here would miss a resumed job.
528+
assert (delivered[0][1].JobKey, delivered[0][1].ResumeVersion) == (JOB_ID, 2)
525529
assert delivered[0][1].Message == "pooled line"
526530

527531
def test_runjob_installs_the_sinks_from_the_request_callback(self, monkeypatch):
@@ -532,7 +536,7 @@ def test_runjob_installs_the_sinks_from_the_request_callback(self, monkeypatch):
532536
monkeypatch.setattr(
533537
_job_api,
534538
"install_runtime_sinks",
535-
lambda jid, cb, loop, rv=None: events.append(("install", jid, cb)),
539+
lambda jid, rv, cb, loop: events.append(("install", jid, cb)),
536540
)
537541
monkeypatch.setattr(
538542
_job_api, "clear_runtime_sinks", lambda: events.append(("clear",))
@@ -574,7 +578,7 @@ async def aflush_pending(self, *a: Any, **k: Any) -> None:
574578
events.append("flush")
575579

576580
monkeypatch.setattr(
577-
_job_api, "install_runtime_sinks", lambda jid, cb, loop, rv=None: _Handler()
581+
_job_api, "install_runtime_sinks", lambda jid, rv, cb, loop: _Handler()
578582
)
579583
monkeypatch.setattr(_job_api, "clear_runtime_sinks", lambda: None)
580584

@@ -611,7 +615,7 @@ def test_runjob_skips_sinks_when_not_opted_in(self, monkeypatch):
611615
monkeypatch.setattr(
612616
_job_api,
613617
"install_runtime_sinks",
614-
lambda jid, cb, loop, rv=None: events.append(("install",)),
618+
lambda jid, rv, cb, loop: events.append(("install",)),
615619
)
616620
monkeypatch.setattr(
617621
_job_api, "clear_runtime_sinks", lambda: events.append(("clear",))

0 commit comments

Comments
 (0)