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

Add configuration storage class. Made DigikeyAPI into a class #15

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Electro707
Copy link

@Electro707 Electro707 commented May 31, 2021

This PR adds a method of setting and saving this package's settings (like client-id and refresh-token) without needing to use OS environment variables.
Main changes includes:

  • Added all API fucntions (for V3) under a DigikeyAPI class.
  • Added a configuration class which is fed into DigikeyAPI. This class includes all necessary functions to be able to save and get configurations. The parent class is DigikeyBaseConfig, which that one does nothing but outline what functions any child class must override. As of right now I've only 1 config storage method/kind under DigikeyJsonConfig, which stores and retrieves configurations in a JSON file.
  • Added the functions needs_client_id(), needs_client_secret(), and set_client_info() to allow the user to know if said info are needed and to be able to give them to this library.

TODO:

  • Document DigikeyBaseConfig further to specify how a user can create their own (for example if someone wants to store said settings inside a database).
  • Document the new changes to how to call up the API
  • Change V2's code to allow for the new configuration:
    • Is this even worth it, or is it better to drop V2 support?
    • V2 support is dropped

@eeintech
Copy link

This is an interesting proposition and I hope to see this being merged.

@peeter123 Any plan to review this soon?

@Electro707 Do you need help with testing?

@Electro707
Copy link
Author

@eeintech Sure, just beware that A) This is not too well documented which is something I need to do, and B) Only works with V3 API. @peeter123 Is it even worth it to add support to v2 even tough it's deprecated by Digikey, or is it time to drop V2 support?

@Electro707 Electro707 changed the title WIP: Add configuration/settings storage class. Add configuration/settings storage class. Sep 26, 2021
@Electro707
Copy link
Author

@peeter123 Any thoughts on the V2 API, if I should add this new config class to it or if it should be deprecated?

@peeter123
Copy link
Owner

I will look into this, I think we can make a 1.0 version where we drop the V2 API completely. What do you think?

@peeter123
Copy link
Owner

Could you rebase on development-1.0 branch in peeter123/digikey-api?

@Electro707
Copy link
Author

Rebasing is done.

@Electro707 Electro707 changed the title Add configuration/settings storage class. Add configuration storage class. Made DigikeyAPI into a class Dec 27, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants