add forward compatibility for byte datagram keys #3045
Conversation
jackrobison
added
type: improvement
jackrobison
changed the title
jackrobison
force-pushed
the
bencode-byte-keys
branch
from
f28a7d1
to
58d61f0
Compare
| } | ||
|
|
||
| if converted[b'0'] in [REQUEST_TYPE, ERROR_TYPE, RESPONSE_TYPE]: # pylint: disable=unsubscriptable-object | ||
| datagram_type = converted[b'0'] # pylint: disable=unsubscriptable-object |
There was a problem hiding this comment.
Does more recent pylint still have this issue? What if you add type information to converted, maybe pylint will detect it as dict then?
| @@ -144,27 +144,37 @@ def __init__(self, packet_type: int, rpc_id: bytes, node_id: bytes, exception_ty | |||
| self.response = response.decode() | |||
|
|
|||
|
|
|||
| def decode_datagram(datagram: bytes) -> typing.Union[RequestDatagram, ResponseDatagram, ErrorDatagram]: | |||
| def _decode_datagram(datagram: bytes): | |||
There was a problem hiding this comment.
i assume that since @jackrobison made this an internal helper function (with underscore) it didn't benefit from having the type info? i think either way is fine: having it doesn't hurt anything, not having it is probably okay in this special case
personally, my preference would be to avoid having such large type Union in the first place, if all of the types have the same parent class, perhaps just say it returns the parent class type? unions should be used as last resort (or when the values returned are truely different, like a function returning an int and a str, in this case it seems it returns Datagrams)
jackrobison
force-pushed
the
bencode-byte-keys
branch
from
58d61f0
to
3a64ceb
Compare
No description provided.