search

MaddieM4 / EJTP-lib-python

Encrypted JSON Transport Protocol library for Python 2.6-3.3.
GNU Lesser General Public License v3.0
23 stars 10 forks source link

Bug137 address class #138

Open MoritzS opened 11 years ago

MoritzS commented 11 years ago

I added the Address class, but many tests are still breaking and I can't find the cause at the moment. Also, I couldn't remove the functions py_address and str_address because ejtp.identity uses them for handling identity names. This must be fixed, too.

MaddieM4 commented 11 years ago

That's pretty much what I was expecting. This is the sort of thing where sometimes it works out of the box as a nice surprise, but usually it breaks a bunch of random crap and takes awhile to clean up afterwards. Let me know when that cleanup is over.

On Sun, Jun 9, 2013 at 2:24 PM, Moritz Sichert notifications@github.comwrote:

I added the Address class, but many tests are still breaking and I can't find the cause at the moment. Also, I couldn't remove the functions py_address and str_address because ejtp.identity uses them for handling identity names. This must be fixed,

too.

You can merge this Pull Request by running

git pull https://github.com/MoritzS/EJTP-lib-python bug137-address-class

Or view, comment on, or merge it at:

https://github.com/campadrenalin/EJTP-lib-python/pull/138 Commit Summary

  • Added Address class in ejtp.address
  • Added test for new Address class
  • frame.address now uses Address class
  • Readded py_address and str_address

File Changes

  • M ejtp/address.pyhttps://github.com/campadrenalin/EJTP-lib-python/pull/138/files#diff-0(48)
  • M ejtp/frame/address.pyhttps://github.com/campadrenalin/EJTP-lib-python/pull/138/files#diff-1(4)
  • M ejtp/tests/test_address.pyhttps://github.com/campadrenalin/EJTP-lib-python/pull/138/files#diff-2(47)

Patch Links:

iurisilvio commented 11 years ago

Just to reference the original issue: #137

MoritzS commented 11 years ago

I need a decision here. Should identity "addresses" treated as real addresses? Because right now they get mixed up heavily. If yes, we need to generalize the Address class a bit (i.e. not calling the fields "addr_type" and such, but something like "type" and "details"). If no, Identity should either define its own "address" class or just use plain lists but without interfering with ejtp.address.

MaddieM4 commented 11 years ago

I would think that the correct course of action is using the Address class in identities. You'll need some code to convert lists into Addresses, but due to their namedtuple heritage, they should be list-y enough to not interfere with JSON serialization. So you only need to worry about catching things on the setter side. On Jun 16, 2013 12:19 PM, "Moritz Sichert" notifications@github.com wrote:

I need a decision here. Should identity "addresses" treated as real addresses? Because right now they get mixed up heavily. If yes, we need to generalize the Address class a bit (i.e. not calling the fields "addr_type" and such, but something like "type" and "details"). If no, Identity should either define its own "address" class or just use plain lists but without interfering with ejtp.address.

— Reply to this email directly or view it on GitHubhttps://github.com/campadrenalin/EJTP-lib-python/pull/138#issuecomment-19517348 .

MaddieM4 commented 10 years ago

I have a few other things I'm going to do first, but if this isn't ready to go in like a week, I will snipe it. I really want to get the 0.9.7 release out, because it has some important improvements in it, and I intend to have Address Objects in the feature list.

MaddieM4 commented 10 years ago

Gonna go back on part of my above statement. There is a lot of work that still needs to be done debugging this change, and it will incur a lot of third-party work to support as well. So I'm going to push it out of the 0.9.7 feature list for the sake of time on both sides - we can introduce and deal with the incompatibilities later.

However, I'm still probably going to end up doing it myself in the somewhat near future, and if that happens, I will revoke the bounty. The only thing changing is the deadline.

rht commented 9 years ago

Hmmm...these commits almost hit the mark, except that

  1. You haven't replaced all the occurrences of str_address and py_address with Address for internal consistency of the data types.
  2. Several of the tests need to be rewritten, e.g. 
frame.encrypted.EncryptedFrame('r["testing",null,null]\x00gpp')

instead of

frame.encrypted.EncryptedFrame('r["testing"]\x00gpp')

.

I just made a version that passes all the unit tests, will ship soon. ;)

MaddieM4 commented 9 years ago

Ooh, hold off, hold off!

Before making any changes, you should know that I am not currently developing EJTP-lib-python anymore. I'm happy to take improvements, if anyone else is using it, but the DJDNS project that motivated me to create this library now uses a completely different language and networking abstraction.

If this library is still interesting and potentially useful to you, I will happily help and pay out the bounty and whatnot. You should just be aware that until other people find a use for it, I consider it abandonware.

On Sun, Oct 26, 2014 at 2:15 AM, rht notifications@github.com wrote:

Hmmm...these commits almost hit the mark, except that

  1. You haven't replaced all the occurrences of str_address and py_address with Address for internal consistency.
  2. Several of the tests need to be rewritten, e.g. frame.encrypted.EncryptedFrame('r["testing",null,null]\x00gpp') instead of just

I just made a version that passes all the unit tests, will ship soon. ;)

— Reply to this email directly or view it on GitHub https://github.com/campadrenalin/EJTP-lib-python/pull/138#issuecomment-60511283 .