KeenClient-Python
KeenClient-Python copied to clipboard
rm pycryptodome
Scoped Keys are deprecated and pycryptodome is not pure python
making it a problem for people wanting to pip install keen
on
some systems. pycryptodome is a dependency of only scoped keys.
To make the tests pass, you'll need to have it installed but that
is enabled via the test_requires within setup.py. A fun error
message is displayed when anything scoped keys are imported. This
may make from keen import *
fail, but that's bad python to begin
with so I don't feel very bad about that.
This includes a documentation update reflecting the manual step required for the deprecated feature.
Testing
Try running the test suite without pycryptodome installed. It should still succeed.
Try import keen.scoped_keys
without pycryptodome installed. It should fail with a message.
Related tickets
#131
Neat. I didn't know about reraise.
On Oct 25, 2017 6:33 PM, "Justin Mason" [email protected] wrote:
@masojus approved this pull request.
Minor comment, but looks fine.
In keen/scoped_keys.py https://github.com/keenlabs/KeenClient-Python/pull/141#discussion_r147022171 :
+try:
- from Crypto.Cipher import AES +except ImportError as ie:
- ie.args = (ie.args[0] + ' -- Scoped Keys are deprecated in favor'
' of Access Keys. To use Scoped Keys anyway, run: '
'`pip install pycryptodome` and try this again.',)
- raise ie
This will blow away the original stack, right? raise ie in python 2 will blow away the stack anyway since you have to use the 3-arg raise, and in python 3, by replacing the entire args tuple we're disregarding any information previously stored on the exception except the message, right? In this case, maybe the message is enough, but if not, we could use the six module's reraise which I think forwards to the future utils in python 2 and uses the raise ... from ... facility in python 3. Just a thought.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/keenlabs/KeenClient-Python/pull/141#pullrequestreview-72052203, or mute the thread https://github.com/notifications/unsubscribe-auth/ACitbywOx3fECfFiv7-BAJdED8O5_DMmks5sv9NLgaJpZM4QDU1s .
I'd like to give 0.5.1
a chance to simmer in the event that I have something broken still and need to push 0.5.2
rather than make progress on 0.6.0
. In particular, I'll want to get better test coverage overall before I'll be satisfied with 0.5.x
moving onward. So I'm holding off, at least for the moment, on merging this.
I am thinking I can also use environment markers and extras_require
here to pick and choose the dependencies in setup.py
. That doesn't mean we don't need the runtime checks, but could make some things easier.