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
Results Ack Sending #571
Results Ack Sending #571
Conversation
Codecov Report
@@ Coverage Diff @@
## main #571 +/- ##
==========================================
+ Coverage 38.76% 38.80% +0.03%
==========================================
Files 27 28 +1
Lines 2920 2974 +54
==========================================
+ Hits 1132 1154 +22
- Misses 1788 1820 +32
Continue to review full report at Codecov.
|
""" Initialize results storage and timing for log updates | ||
""" | ||
self.unacked_results = {} | ||
self.unacked_count_log_period = 60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit confusing, maybe just log_period
is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks mostly good to go. I've added some comments where renaming the variables would improve clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, this looks good to go.
[ch10083] |
This pull request has been linked to Clubhouse Story #10083: Implement ACK as part of forwarder protocol. |
Description
See funcx-faas/funcx-forwarder#27
Type of change