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

Create very simple RPC so the subprocess loads functions #584

Merged
merged 8 commits into from
Aug 14, 2024

Conversation

callumforrester
Copy link
Contributor

@callumforrester callumforrester commented Aug 2, 2024

Fixes #590

Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 95.34884% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.84%. Comparing base (5351741) to head (b5f537d).

Files Patch % Lines
src/blueapi/service/runner.py 94.11% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #584      +/-   ##
==========================================
- Coverage   92.86%   92.84%   -0.02%     
==========================================
  Files          40       40              
  Lines        1793     1818      +25     
==========================================
+ Hits         1665     1688      +23     
- Misses        128      130       +2     

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

@callumforrester callumforrester marked this pull request as ready for review August 5, 2024 10:43
Copy link
Contributor

@DiamondJoseph DiamondJoseph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving for the _rpc function, if @callumforrester can approve for the other changes I made ;)

@joeshannon
Copy link
Contributor

Is there an associated issue for this? I can't quite infer what the problem was from this PR. Presumably pickle related?

@DiamondJoseph
Copy link
Contributor

I can't quite infer what the problem was from this PR. Presumably pickle related?

I assume so from Callum's first commit (but surely still has to pickle the args, kwargs just not the function?), my interest in it is it makes upcoming observability handling easier

@callumforrester
Copy link
Contributor Author

I've made a retroactive issue: #590

@DiamondJoseph DiamondJoseph merged commit fd41e57 into main Aug 14, 2024
24 checks passed
@DiamondJoseph DiamondJoseph deleted the dont-pickle-functions branch August 14, 2024 10:38
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.

Subprocess functions can't be picked when decorated for observability
3 participants