This gets rid of some non-type safe string comparisons and access to members that are implementation details of class P2PConnection(asyncore.dispatcher). Such refactoring is required to replace the deprecated asyncore with something more sane.
Changes:
state and replace is with bool connectedpushbuf and literally just push to the buffer at the call site76@@ -77,14 +77,19 @@ def __init__(self):
77
78 super().__init__(map=mininode_socket_map)
79
80+ self.conn_open = False
81+
82+ def is_connected(self):
@property decorator)
self._transport is not None or self._state == State.CONNECTED or …
@property decorated method can do anything any other method can do.
P2PConnection.state from a string and making it private, however I think it’d be a cleaner implementation to just turn state into an Enum class rather than two bools (asyncore_pre_connection and conn_open). Also rename to _state to indicate that it’s private.
conn_open) and a bool can represent all states we are interested in. The asyncore_pre_connection is an implementation detail of asyncore and will disappear the same time asyncore disappears. Imo, no need to artificially introduce an enum and then flatten it again to a bool in a follow up commit.
no need to artificially introduce an enum and then flatten it again
I disagree that this is artificially introducing an enum. P2PConnection.state already is an enum, just implemented in a really bad way :) why not just wait to flatten it to a bool when the P2PConnection implementation is changed to asyncio or whatever?
tbh though, this is just nitting. Fine to split this to two bools now if that’s what you prefer. I would suggest you name them _conn_open and _asyncore_pre_connection to indicate that they’re private.
p2p_sendheaders.py fails. I have logs from a local failure if you want them.
No need to ask for permission. Just throw them at me
On Thu, Jun 21, 2018, 17:29 John Newbery notifications@github.com wrote:
p2p_sendheaders.py fails. I have logs from a local failure if you want them.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/13512#issuecomment-399249293, or mute the thread https://github.com/notifications/unsubscribe-auth/AGGmv0vzYfGiS1Xd-kCvjPDwTpowq3Qbks5t_BBKgaJpZM4UxRTg .
Labels
Tests