Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File outputs #54

Merged
merged 14 commits into from
Nov 28, 2023
Merged

File outputs #54

merged 14 commits into from
Nov 28, 2023

Conversation

rwb27
Copy link
Collaborator

@rwb27 rwb27 commented Nov 28, 2023

I've added a BlobOutput class that makes file outputs from actions much neater. For now, actions may declare their return type as a subclass of BlobOutput, having set the media_type property. The function must then return a BlobOutput instance, which will appear as a link in the output member of an action invocation.

The link will point to /invocations/{id}/output/ and, when followed, will return the output directly.

Future extensions might include:

  • Multiple file outputs (probably via a customised Pydantic model)
  • Returning the output directly if an action completes within a timeout (i.e. actually implementing the 200 response from action invocations)
  • Handling specific media types nicely in client code (e.g. helpfully offering to convert images using PIL).

rwb27 added 13 commits November 27, 2023 16:49
ThingClient subclasses can now be initialised from URLs.

thing_client_from_url is now ThingClient.from_url
NB these are both untested and unused.
Actions may now declare a return type derived from
BlobOutput. This will serialise to a JSON link specifying the
media type, and will allow us to return a raw (can be binary)
response.

In the future, we should use the `accept` HTTP header to pick
whether to return JSON or the specified mime type. We
should also implement behaviour that makes the
200 status code reachable from an action invocation.
The HTTP client now checks whether the output of an
action is a blob. If so, it will return a ClientBlobOutput that
downloads it and either saves to a file or returns as bytes.
This should do a complete test of blob output - using both the
temp dir based and bytes-based methods of constructing it,
and retrieving it via the three specified methods.
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (8f16e6f) 75.25% compared to head (caa7f7a) 78.70%.

Files Patch % Lines
src/labthings_fastapi/outputs/blob.py 89.42% 11 Missing ⚠️
src/labthings_fastapi/client/__init__.py 92.30% 2 Missing ⚠️
src/labthings_fastapi/descriptors/action.py 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #54      +/-   ##
==========================================
+ Coverage   75.25%   78.70%   +3.44%     
==========================================
  Files          26       28       +2     
  Lines        1475     1620     +145     
==========================================
+ Hits         1110     1275     +165     
+ Misses        365      345      -20     
Flag Coverage Δ
unittests 78.70% <90.85%> (+3.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@rwb27
Copy link
Collaborator Author

rwb27 commented Nov 28, 2023

Test coverage of the diff isn't 100%, but most of the missed lines are ones I don't expect we'll need often or at all. Improving coverage is always a good idea, but I'm going to prioritise other things for now.

File outputs work, and are tested - we will need to add the ability to handle particular media types more nicely (e.g. convert JPEG images into PIL objects or arrays), but the basics seem good for now.

@rwb27 rwb27 merged commit 151f7ac into main Nov 28, 2023
5 checks passed
@rwb27 rwb27 deleted the file_outputs branch November 28, 2023 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant