qa: Only allow disconnecting all NodeConns #11641
Conversation
| p2p_conns = [] | ||
|
|
||
| for i in range(3): | ||
| for _ in range(3): |
There was a problem hiding this comment.
Interested in the use of the underscore as the variable name here, did some reading and from what I've read use of the underscore only works as something other than a variable name when used in an interactive python shell? Other than that people seem to believe it's a bad naming convention.
There was a problem hiding this comment.
_ as a variable name in python is a convention to indicate that the result won't be used.
There was a problem hiding this comment.
Thanks for the explanation, makes sense.
| """Close all p2p connections to the node.""" | ||
| for _ in range(len(self.p2ps)): | ||
| index = 0 | ||
| # Connection could have already been closed by other end. |
There was a problem hiding this comment.
Is the order of deletion relevant? If not, something like this is both easier to read and likely more efficient:
for x in self.p2ps:
if x.connection is not None:
x.connection.disconnect_node()
self.p2ps = []
There was a problem hiding this comment.
No, the order is not relevant. Will switch to your suggestion. (I tried to minimize the --ignore-all-space diff)
maflcko
force-pushed
the
Mf1711-qaNodeConnDisconnectAll
branch
2 times, most recently
from
fa3408a to
fa3a437
Compare
maflcko
force-pushed
the
Mf1711-qaNodeConnDisconnectAll
branch
from
fa3a437 to
faaa7db
Compare
|
utACK faaa7db |
faaa7db qa: Only allow disconnecting all NodeConns (MarcoFalke) Pull request description: Disconnecting the connection with `index=0` makes no sense when there are more than one connections, as the list "rotates around" and populates index 0 after `del`. Just disconnect all NodeConns in any case. Tree-SHA512: e5cf540823fccb31634b5a11501f54222be89862e80ccafc28bc06726480f8d2153b8c1b6f859fa6a6d087876251d48a6c6035bccdaaf16831e300bc17ff613d
faaa7db qa: Only allow disconnecting all NodeConns (MarcoFalke) Pull request description: Disconnecting the connection with `index=0` makes no sense when there are more than one connections, as the list "rotates around" and populates index 0 after `del`. Just disconnect all NodeConns in any case. Tree-SHA512: e5cf540823fccb31634b5a11501f54222be89862e80ccafc28bc06726480f8d2153b8c1b6f859fa6a6d087876251d48a6c6035bccdaaf16831e300bc17ff613d
4 participants
Disconnecting the connection with
index=0makes no sense when there are more than one connections, as the list "rotates around" and populates index 0 afterdel.Just disconnect all NodeConns in any case.