Conversation
Barecheck - Code coverage reportTotal: 96.31%Your code coverage diff: -0.21% ▾ Uncovered files and lines |
|
Not entirely sure how this MR closes #145? It appears this context variable was removed before this MR. |
|
I see in the PR description you've put 'I've checked that this doesn't appear to break the OpenFlexure Microscope Server unit or integration tests, as of b6324e9' - It would be good to see some evidence of this. Should we wait until the PR more putting OFM tests in CLI is merged before merging this? Just to be on the safe side. |
|
Final note - I'm not worried about test coverage going down 0.21%. The deleted tests only refer to deleted dependency and blob_type functionality. |
I'd put that on the critical path for release rather than merging each MR. |
Ah, at the point when I put that in I had already deleted the invocation-related dependencies, and mistakenly thought it was in that module. Nice to know it was fixed already. |
As I wrote that, I realised it was almost inevitable that commits would be added and that statement would then become incorrect... #280 is now passing, so I propose we merge it and rebase this, then merge this once we're happy. |
These tests cover code that will be deleted in this PR.
This commit removes dependencies related to Invocations. It moves generation of the invocation ID into `ActionManager.invoke_action`.
This is replaced by functionality in `Thing._thing_server_interface`.
This is replaced by functionality in `ThingServerInterface`.
This gets rid of a bunch of now-obsolete code that used dependencies for inter-thing communication. The new `lt.thing_slot` system does this much better. This required removing a section of the docs and its example code, as that was being tested. That's helpful, as said section detailed the features I'm removing. I have retained the `_dependencies` reference in Sphinx but moved it to a new section on removed features: this should help redirect people to how the code has changed.
blob_type is deprecated in favour of subclassing `Blob` explicitly.
Co-authored-by: Beth <167304066+bprobert97@users.noreply.github.com>
julianstirling
left a comment
julianstirling
left a comment
There was a problem hiding this comment.
I note that @bprobert97's comments have been acted on. Great to see the OFM tests running on this with no problems (as expected, but nice to see).
This is a very very useful clean up of the repository. Delighted to accept it.
3 participants
This PR gets rid of dependencies, included the much-disliked
DirectThingClient. It also removesblob_type.The code that's deleted is:
labthings_fastapi.deps(a convenience-import module for dependencies)labthings_fastapi.dependencies.*(includes a bunch of submodules)labthings_fastapi.blob.blob_type(function that returns aBlobsubclass)I've also changed the signature of
ActionManager.invoke_actionand the endpoint that starts an action, to remove the invocation ID dependency. Invocation IDs are now generated inActionManager.invoke_action.I've removed the documentation on dependencies, and had a quick skim of the docs to check for anything obsolete in there.
I've checked that this doesn't appear to break the OpenFlexure Microscope Server unit or integration tests, as of b6324e9
Closes #196
Closes #128
Closes #122