feat: Include plan outcome in WorkerEvents

[tpoliaw]

The TaskStatus included in each WorkerEvent now contains a result
field that will be populated when the plan is complete. This can be
either a TaskResult that contains a serialized representation of the
return value from a plan that succeeded, or a TaskError containing the
Exception type and message raised by a plan that failed.

The existing task_complete and task_failed flags have been left for
backwards compatibility but are now redundant and may be removed in a
future version.

Error reporting in WorkerEvents is left unchanged for now but may also
change in future as plan errors are included in the worker error list
making it unclear where an error occurred.

The new outcome states in the returned TaskStatus allows better
feedback in the CLI and errors/return values are included in the output.

The BlueapiClient run_task method now returns only the TaskStatus as it
now contains all relevant information about the plan. A plan failing
will now not raise an exception with exceptions being raised only in the
case of the communication errors or internal server errors. Users of the
client should check if plans were successful.

[tpoliaw]

This would need a matching change in GDA (back-ported to 9.38?) to add the result type to TaskStatus here.

[tpoliaw]

Ignoring the GDA change, I think the blueapi side of this is workable

[tpoliaw]

Change in gerrit: https://gerrit.diamond.ac.uk/c/gda/gda-core/+/44673

[DominicOram]

Looks good, thanks. Should: it would be nice to add some tests on the case where the plan is actually returning something, ideally parametrized against a few different return types.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.01%. Comparing base (fe35430) to head (094a152).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1357      +/-   ##
==========================================
- Coverage   95.03%   95.01%   -0.02%     
==========================================
  Files          43       43              
  Lines        2779     2829      +50     
==========================================
+ Hits         2641     2688      +47     
- Misses        138      141       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tpoliaw]

Putting this back to draft so #1312 can be included in the result

[dan-fernandes]

Minor changes / questions, everything else looks reasonable 👍

[dan-fernandes]

What is the benefit of using cast instead of assuming obj["client"] will be of type BlueapiClient and just typing it as such?

[tpoliaw]

For some reason the type-checker fails to recognise that it is a BlueapiClient otherwise. Using typing.reveal_type gives

$ uv run pyright src/blueapi/cli/cli.py                                                                      17:01:39
/dls/athena/blueapi/src/blueapi/cli/cli.py
  /dls/athena/blueapi/src/blueapi/cli/cli.py:295:17 - information: Type of "client" is "Unknown | BlueapiClient"

It might be ok to just use the cast though without the type-hint

[dan-fernandes]

Is the success / failure of the task logged anywhere? I am open to print statements in a CLI for a human user, but I think that either:

• These should be logs, or
• TaskWorker should log outcome of task

[tpoliaw]

Are logs from the CLI sent anywhere other than the terminal? I have added a success/fail log to the task worker but that won't affect the client output.

Arguably these should use click.echo but it used print before and I didn't want to change that at the same time.

[tpoliaw]

Good spot on the unused loggers - they were left from an earlier iteration. Thanks