google / keyczar

Easy-to-use crypto toolkit
Apache License 2.0
1.1k stars 141 forks source link

Python 3 and other #164

Closed jbtule closed 3 years ago

jbtule commented 9 years ago

This was the last pull request I had from 2 years ago adding python 3 support. It's only difference from jbtule/keyczar-python2to3 is it doesn't have the Travis-CI continuous integration, since that is per repository and there are multiple platforms in the google keyczar repository. Do with it what you will, I imagine 2 years will have some adjustments needed.

devinlundberg commented 9 years ago

I skimmed across this PR for about an hour today and overall I really like how it reorganizes the code and makes the split between binary data and strings more concrete.

From a high level I noticed: 1) The indentation seems to be inconsistent especially when continuing lines. @divegeek I have been writing lots of python using pep8 for the past year now, could you refresh me on the google python style guide? (If they open sourced any of their style tools that would also be a huge help) 2) I feel like the submodules don't really fit with anything else we have in our repositories. We can probably get rid of the javatests if we get this working with the interop testing. 3) I should probably give the stream sections and the testing for those sections much closer looks since those seem to be the biggest changes in actual logic. 4) There are definitely going to be rebasing issues since I noticed conflicts with several more recent inclusions to the python project (like optional encoders). Hopefully these won't take too long.

@jbtule I'll probably end up forking this and taking off where you left off since you said you don't have time to work on this. If that's not the case, let me know. Otherwise I'll add you on the PRs and would be happy to have your feedback.

jbtule commented 9 years ago

@devinlundberg fork away!

dgryski commented 9 years ago

@devinlundberg maybe run https://github.com/google/yapf across the whole code base?

devinlundberg commented 9 years ago

@dgryski looks like a useful tool! I'll play around with it.

bufke commented 9 years ago

I was playing around with this. Did RsaPublicKey.Encrypt become RsaPublicKey.EncryptIO? Is this intentional? It seems inconvenient having to use BytesIO if I just want to encrypt a string in memory.

jbtule commented 9 years ago

You don't use the encrypt functions of the keys directly, because if you do you are bypassing one of the biggest security features of keyczar, the key rotation. So design wise using byte streams on the non public API portion makes a lot of sense. There is still a string version of encrypt method on the public facing Encrypter class.

soferio commented 8 years ago

Where does this fit in? https://pypi.python.org/pypi/python3-keyczar/0.71rc0

jbtule commented 8 years ago

@soferio It looks like someone added this pull request to pypi. I just eye-balled it and saw most of my changes in it.

asacamano commented 8 years ago

It seems like there's some real value here - but there are conflicts... Anyone want to finish this off?