EJTP-lib-python icon indicating copy to clipboard operation
EJTP-lib-python copied to clipboard

Bug137 address class

Open MoritzS opened this issue 12 years ago • 8 comments

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.

MoritzS avatar Jun 09 '13 21:06 MoritzS

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 [email protected]:

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:

  • https://github.com/campadrenalin/EJTP-lib-python/pull/138.patch
  • https://github.com/campadrenalin/EJTP-lib-python/pull/138.diff

MaddieM4 avatar Jun 10 '13 05:06 MaddieM4

Just to reference the original issue: #137

iurisilvio avatar Jun 12 '13 12:06 iurisilvio

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.

MoritzS avatar Jun 16 '13 19:06 MoritzS

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" [email protected] 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 avatar Jun 16 '13 20:06 MaddieM4

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 avatar Nov 02 '13 18:11 MaddieM4

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.

MaddieM4 avatar Nov 11 '13 05:11 MaddieM4

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. ;)

rht avatar Oct 26 '14 09:10 rht

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 [email protected] 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 .

MaddieM4 avatar Oct 26 '14 16:10 MaddieM4