search

crossbario / autobahn-python

WebSocket and WAMP in Python for Twisted and asyncio
https://crossbar.io/autobahn
MIT License
2.47k stars 763 forks source link

Allow tests to pass without XBR dependencies #1580

Closed yan12125 closed 2 years ago

yan12125 commented 2 years ago

Closes #1579

oberstet commented 2 years ago

great, thanks! only 2 minors comments / possible improvements you might try (pls see above) ..

yan12125 commented 2 years ago

only 2 minors comments / possible improvements

Hmm, I didn't see comments other than this one in this pull request. Did you leave them somewhere else?

oberstet commented 2 years ago

Hmm, I didn't see comments other than this one in this pull request. Did you leave them somewhere else?

I did "start a review" and added comments to that - this is I can see my comments exactly above (not sure why you don't see it? who knows ..) let me Escher-Paste:

grafik

yan12125 commented 2 years ago

I did "start a review" and added comments to that - this is I can see my comments exactly above

Aha, you'll need to hit "Finish your review" to make those comments public :)

InvalidPayload should not reside under the import guard

Thanks! I moved all such cases out.

rgd CryptosignKey, this should reside under the from autobahn.wamp.cryptosign import HAS_CRYPTOSIGN

Yes, that makes sense! I changed to the approach used by current autobahn/xbr/test/test_xbr_secmod.py and put HAS_XBR and HAS_CRYPTOSIGN together. I noticed that autobahn[xbr] installs pynacl, so HAS_CRYPTOSIGN is always True when HAS_XBR is True. It may not be useful to have two separate checks.

yan12125 commented 2 years ago

Thanks for fast responses and constructive comments!

Now I can see review comments, by the way :)

oberstet commented 2 years ago

great, thanks for contributing! ah, and yes, I failed to hit "finish review", so that explains that;)