Reject misplaced empty `runtime_kwargs`
Treat `runtime_kwargs` as provided whenever it is not `None`. Previously an empty dict bypassed placement validation and was silently ignored when `an` or `portal` selected an existing runtime. Reject both placement modes before actor startup for empty and configured runtime kwargs while preserving empty-dict use when `to_actor.run()` owns its private runtime. Caught-during: review remediation Found-via: `/code-review` P3 option-validation finding Review: PR #481 (opencode) https://github.com/goodboy/tractor/pull/481#pullrequestreview-4956692120 Prompt-IO: ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.md (this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`))wkt/to_actor_subpkg
parent
fe0a724d10
commit
20e89334c0
|
|
@ -0,0 +1,34 @@
|
|||
---
|
||||
model: openai/gpt-5.6-sol
|
||||
service: opencode
|
||||
session: ses_3e4c90d3eafeqHEtRYSIHgHhpA
|
||||
timestamp: 2026-08-19T02:07:57Z
|
||||
git_ref: b38efed7
|
||||
scope: code
|
||||
substantive: true
|
||||
raw_file: 20260819T020757Z_b38efed7_prompt_io.raw.md
|
||||
---
|
||||
|
||||
## Prompt
|
||||
|
||||
Resolve the remaining P3 review finding before landing PR #481:
|
||||
`runtime_kwargs={}` must not be silently accepted alongside either
|
||||
`an=` or `portal=` merely because the dict is falsey. Keep this as its
|
||||
own final review-remediation commit.
|
||||
|
||||
## Response summary
|
||||
|
||||
Treat any non-`None` `runtime_kwargs` value as provided when validating
|
||||
placement. Cover both placement APIs with empty and configured dicts,
|
||||
proving the error is raised locally before actor startup.
|
||||
|
||||
## Files changed
|
||||
|
||||
- `tractor/to_actor/_api.py` - validate option presence explicitly.
|
||||
- `tests/test_to_actor.py` - cover four invalid option combinations.
|
||||
|
||||
## Human edits
|
||||
|
||||
No direct line edits. The human accepted the P3 finding, required it to
|
||||
remain separate from the five P2 behavioral commits and prioritized it
|
||||
before the final PR #484 integration rebase and PR #481 landing steps.
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
---
|
||||
model: openai/gpt-5.6-sol
|
||||
service: opencode
|
||||
timestamp: 2026-08-19T02:07:57Z
|
||||
git_ref: b38efed7
|
||||
diff_cmd: git diff HEAD~1..HEAD
|
||||
---
|
||||
|
||||
Fix the final PR #481 review finding: `runtime_kwargs` is mutually
|
||||
exclusive with both caller placement options whenever it is provided,
|
||||
including an empty dict.
|
||||
|
||||
> `git diff HEAD~1..HEAD -- tractor/to_actor/_api.py tests/test_to_actor.py`
|
||||
|
||||
Use an explicit `is not None` check rather than dict truthiness. Expand
|
||||
the validation regression across `an=` and `portal=`, each with empty
|
||||
and configured runtime kwargs, so every invalid combination fails
|
||||
before actor runtime startup.
|
||||
|
||||
Verification:
|
||||
|
||||
- Trio/TCP: `23 passed`
|
||||
- Trio/UDS: `23 passed`
|
||||
- `mp_spawn`/TCP: `23 passed`
|
||||
- Ruff and `git diff --check`: clean
|
||||
|
|
@ -511,12 +511,30 @@ def test_rejects_portal_and_an_combo():
|
|||
)
|
||||
|
||||
|
||||
def test_rejects_runtime_kwargs_with_placement():
|
||||
@pytest.mark.parametrize(
|
||||
'placement',
|
||||
['an', 'portal'],
|
||||
)
|
||||
@pytest.mark.parametrize(
|
||||
'runtime_kwargs',
|
||||
[
|
||||
{},
|
||||
{'loglevel': 'cancel'},
|
||||
],
|
||||
ids=['empty', 'configured'],
|
||||
)
|
||||
def test_rejects_runtime_kwargs_with_placement(
|
||||
placement: str,
|
||||
runtime_kwargs: dict,
|
||||
):
|
||||
'''
|
||||
`runtime_kwargs` only applies when the call opens
|
||||
its own private actor-nursery; passing it alongside
|
||||
a placement opt is an error, never silently
|
||||
ignored.
|
||||
ignored. In particular, an empty dict still means the
|
||||
caller provided this mutually exclusive option; testing
|
||||
both placement modes prevents truthiness checks from
|
||||
accepting it before any actor runtime is started.
|
||||
|
||||
'''
|
||||
with pytest.raises(ValueError):
|
||||
|
|
@ -525,10 +543,10 @@ def test_rejects_runtime_kwargs_with_placement():
|
|||
to_actor.run,
|
||||
add_one,
|
||||
1,
|
||||
an=object(),
|
||||
runtime_kwargs=dict(
|
||||
loglevel='cancel',
|
||||
),
|
||||
**{
|
||||
placement: object(),
|
||||
'runtime_kwargs': runtime_kwargs,
|
||||
},
|
||||
)
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -295,7 +295,7 @@ async def run(
|
|||
fn, args, kwargs = _normalize_call(fn, args)
|
||||
|
||||
if (
|
||||
runtime_kwargs
|
||||
runtime_kwargs is not None
|
||||
and
|
||||
(
|
||||
an is not None
|
||||
|
|
|
|||
Loading…
Reference in New Issue