Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move algorithm-specific logic to be class-based to allow for better extensibility #70

Closed
mark-adams opened this issue Jan 6, 2015 · 4 comments

Comments

@mark-adams
Copy link
Contributor

In #42, we discussed changing to more of a registry model for registration of algorithms. This issue is suggesting that we move to that sort of a model.

mark-adams added a commit to mark-adams/pyjwt that referenced this issue Jan 6, 2015
…e classes in the algorithms module

Added register_algorithm to add new algorithms.
@wbolster
Copy link
Contributor

wbolster commented Jan 6, 2015

As an alternative, setuptools extension points could be used instead. With the current approach, any external "extension package" containing an additional algorithm implementation must be imported by the application before it could be used, because the extension package would register the algorithms only at import time.

With extension points, which are designed for use cases like these, this isn't needed anymore. A simple pip install pyjwt pyjwt-my-supercool-new-crypto-algo-addon would suffice.

The stevedore package takes care of most of the required plumbing; see http://stevedore.readthedocs.org/en/latest/index.html for details and examples. The "driver" approach would work for the use cases outlined in this issue, I think.

@mark-adams
Copy link
Contributor Author

That's a neat idea! I could see that being super useful in applications that see a lot of plug-ins being developed (things like Django, SqlAlchemy, flask) and I have some ideas for where I want to use it. Thanks for sharing it! =)

For this library, I anticipate that 99.9999% of the use-cases will be people who are only using the algorithms listed in the JWA spec (which we implement all of the required, all of the reccomended, and most of the optional). I think that someone registering their own non-standard algorithms with this library would be very uncommon so I'm not sure it justifies the additional dependency on stevedore.

I wouldn't be opposed to the setuptools option though. I'll have to read up on it.

But that's just my two cents :-)

@wbolster
Copy link
Contributor

wbolster commented Jan 6, 2015

Actually I tend to agree with you here @mark-adams. The ugly import-time side-effect and the resulting seemingly "unused" import in application code is just the price one has to pay for using non-standardized crypto! :)

@wbolster
Copy link
Contributor

wbolster commented Jan 7, 2015

For future reference: follow-up discussion in #71.

mark-adams added a commit to mark-adams/pyjwt that referenced this issue Jan 7, 2015
…e classes in the algorithms module

Added register_algorithm to add new algorithms.
mark-adams added a commit to mark-adams/pyjwt that referenced this issue Jan 8, 2015
…e classes in the algorithms module

Added register_algorithm to add new algorithms.
jpadilla added a commit that referenced this issue Jan 18, 2015
Fixes #70. Implemented registry pattern for class-based algorithms
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants