Repository navigation
feat: Support *args and **kwargs in plan definitions - #1715
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1715 +/- ##
==========================================
+ Coverage 96.42% 96.46% +0.03%
==========================================
Files 49 49
Lines 3749 3791 +42
==========================================
+ Hits 3615 3657 +42
Misses 134 134 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
oliwenmandiamond
left a comment
There was a problem hiding this comment.
This looks really good, thank you! I think this is roughly the size of my change though if I didn't add any test logic. But I think this will ultimately be smaller as you don't need to test for V1 and V2 any more, just args, kwargs and args and kwargs. This does look cleaner without V1 and V2 everywhere
(Also really good to have you back Peter)
e92ce04 to
fdb2c2c
Compare
abbiemery
left a comment
There was a problem hiding this comment.
I have tested this against the same sets of plans and mix of args and kwargs as the other pr and all behaves as expected, thank you both for your hard work on this :)
I will make an issue for the potential renaming of params to kwargs, and for the cli syntax.
There is one test in the system test failing just because the match on the error message isn't full compared to what it expects:
FAILED tests/system_tests/test_blueapi_client.py::test_plan_argument_errors - AssertionError: Regex pattern did not match.
Expected regex: "multiple values for 'detectors'"
Actual message: "Incorrect parameters supplied\n multiple values for argument 'detectors'"
But otherwise looks good to go for me.
22a3f3e to
bb4b7a0
Compare
Support *args and **kwargs in plans by binding a provided list of args to parameters before handing keyword arguments off to the generated pydantic model for the plan. TaskRequest has a new `args` field but is optional so existing requests should have no change in behaviour. The `params` field of Task and TaskRequest has been left as params instead of the arguably more sensible kwargs to prevent any changes to existing APIs. The CLI run command has gained an `-a/--args` flag to pass positional arguments. These should be a single string that deserialises to a JSON array. The existing behaviour of optionally passing a JSON object/mapping as keyword arguments has been left unchanged for now. The scripting client interface now fully delegates its argument binding to the server and passes args/kwargs straight through. Co-authored-by: oliwenmandiamond <136330507+oliwenmandiamond@users.noreply.github.com>
Support *args and **kwargs in plans by binding a provided list of args to parameters before handing keyword arguments off to the generated pydantic model for the plan. TaskRequest has a new `args` field but is optional so existing requests should have no change in behaviour. The `params` field of Task and TaskRequest has been left as params instead of the arguably more sensible kwargs to prevent any changes to existing APIs. The CLI run command has gained an `-a/--args` flag to pass positional arguments. These should be a single string that deserialises to a JSON array. The existing behaviour of optionally passing a JSON object/mapping as keyword arguments has been left unchanged for now. The scripting client interface now fully delegates its argument binding to the server and passes args/kwargs straight through. Co-authored-by: oliwenmandiamond <136330507+oliwenmandiamond@users.noreply.github.com>
Support *args and **kwargs in plans by binding a provided list of args to parameters before handing keyword arguments off to the generated pydantic model for the plan. TaskRequest has a new `args` field but is optional so existing requests should have no change in behaviour. The `params` field of Task and TaskRequest has been left as params instead of the arguably more sensible kwargs to prevent any changes to existing APIs. The CLI run command has gained an `-a/--args` flag to pass positional arguments. These should be a single string that deserialises to a JSON array. The existing behaviour of optionally passing a JSON object/mapping as keyword arguments has been left unchanged for now. The scripting client interface now fully delegates its argument binding to the server and passes args/kwargs straight through. Co-authored-by: oliwenmandiamond <136330507+oliwenmandiamond@users.noreply.github.com>
Support *args and **kwargs in plans by binding a provided list of args to parameters before handing keyword arguments off to the generated pydantic model for the plan.
TaskRequest has a new
argsfield but is optional so existing requests should have no change in behaviour. Theparamsfield of Task and TaskRequest has been left as params instead of the arguably more sensible kwargs to prevent any changes to existing APIs.The CLI run command has gained an
-a/--argsflag to pass positional arguments. These should be a single string that deserialises to a JSON array. The existing behaviour of optionally passing a JSON object/mapping as keyword arguments has been left unchanged for now.The scripting client interface now fully delegates its argument binding to the server and passes args/kwargs straight through.
Co-authored-by: oliwenmandiamond 136330507+oliwenmandiamond@users.noreply.github.com