search

mwasilak / txThings

CoAP library for Twisted
94 stars 43 forks source link

Enhancement RFC: Used namedtuple for remote #12

Open jlitzinger-kinestral opened 8 years ago

jlitzinger-kinestral commented 8 years ago

Apologies, I'm hijacking the Issues tab to discuss a feature proposal...not sure if that's cool or not, but it's what they offer. Anyway, as I use txThings (which I like, nicely done), I noticed that storing the remote in a namedtuple with host/port fields might read a little easier. Thoughts?

E.g. Assuming a remote of ::1:5683 and a message object named request IS: 

request.remote = ('::1', 5683)
host = request.remote[0]
port = request.remote[1]

Proposed: A named tuple such that:

Remote = collections.namedtuple('Remote', ['host', 'port'])
request.remote = Remote('::1', 5683)
host = request.remote.host
port = request.remote.port

Additionally, the above index syntax should still work, maintaining backward compatibility for existing clients.

Certainly not required, but conceptually more direct (to me) than [0], [1]. If you are not opposed, I'll make the change and send a PR. In that case, let me know whether you'd like existing uses modified.

mwasilak commented 8 years ago

Seems sensible. Please note that I changed remote tuple to store host address as ipaddress.ip_address and not as simple string (Issue #10).

jlitzinger-kinestral commented 8 years ago

Hmm, I knew that, and forgot prior to writing this suggestion, sorry. Let me review that change and re-evaluate my suggestion above. The ipaddress module may have already achieved the goal I suggested above.

jlitzinger-kinestral commented 8 years ago

Bit of a tangential question, was there a reason you selected py2-ipaddress over ipaddress (https://github.com/phihag/ipaddress)?

I have no experience or preference for one or the other, however, Twisted (optionally) requires cryptography, the latest version of which requires ipaddress. Since you target Twisted specifically I wondered if there was a deliberate reason you chose py2-ipaddress.

I suspect they are similar, but, there is a difference as noted on the py2-ipaddress page. Specifically, ipaddress requires unicode objects vs str objects for constructing ipaddresses, whereas py2-ipaddress accepts both. Additionally, py2-ipaddress uses bytearrays for the binary representation, whereas ipaddress uses strs.

I only bring this up because installing py2-ipaddress, at least for me, overwrites the dependency installed by cryptography, which means if one installs txThings in a working Twisted install that uses cryptography, it has silently affected the cryptography module..which is probably not desirable. It may be harmless, I haven't done any verifications at all.