-
Notifications
You must be signed in to change notification settings - Fork 284
Conversation
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.
Looks good to me!
One small question, will approve on reply or fix :)
continue | ||
if session.task_computer is not None: | ||
session.task_computer.session_timeout() | ||
session.dropped() |
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.
Will dropping the session here change the list of all_sessions
? Think it was 2 loops for this reason
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.
Actually all_sessions
is a frozenset()
thus it can't change. Also it's a dynamically generated property which is a copy of incoming and outgoing sessions sets combined.
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.
nice! ok, approving :D
a506014
to
83181ea
Compare
Codecov Report
@@ Coverage Diff @@
## develop #3896 +/- ##
===========================================
- Coverage 87.77% 87.72% -0.05%
===========================================
Files 216 216
Lines 18941 19081 +140
===========================================
+ Hits 16625 16739 +114
- Misses 2316 2342 +26 |
4229200
to
1c8ceca
Compare
depends on: golemfactory/golem-messages#321
task_id
from TaskSession