Skip to content

Conversation

hartikainen
Copy link
Contributor

Thanks for open-sourcing the package! I just gave it a spin and it looks promising. I found the use of dotenv a bit clumsy and implemented a workaround to use system environment variables. I thought I'd open a PR here in case someone else also finds it useful.

The new logic is as follows:

  • if env is not None, then load_env_variables will load the variables from path env with dotenv.get_key,
  • if env is None, then load_env_variables will load the variables from system environment.

Both cases will check that both environment variables exist.

Also, I was wondering: would it make sense to call these ONSHAPE_{ACCESS,SECRET}_KEY instead of just {ACCESS,SECRET}_KEY?

@senthurayyappan senthurayyappan self-assigned this Jan 13, 2025
@senthurayyappan
Copy link
Member

Thank you for the PR @hartikainen, this is a great idea and a helpful addition.

Yes! calling the keys ONSHAPE_{ACCESS,SECRET}_KEY instead of ACCESS,SECRET}_KEY would save us some trouble in the future. I created a new branch with your PR and some minor changes to the load_env helper functions, can you please help test the loading from system's env use case? Thanks again.

@hartikainen
Copy link
Contributor Author

Thanks @senthurayyappan. I can confirm that my example from your branch works both with .env and system environment variables.

@senthurayyappan
Copy link
Member

Thank you for confirming, moving our conversation to the updated branch and PR: #33

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.

2 participants