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

Switch environment variables to .env, and make direnv optional #28

Closed
avisionh opened this issue May 28, 2021 · 9 comments
Closed

Switch environment variables to .env, and make direnv optional #28

avisionh opened this issue May 28, 2021 · 9 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@avisionh
Copy link
Contributor

avisionh commented May 28, 2021

I haven't tested this out myself so unsure if it will work but it sounds like you can replace the line in the .envrc file here with dotenv.

direnv/direnv#284 (comment)

Adds an extra dependency but you'll need this dependency anyways if you want to use the .envrc framework within PyCharm.

@ESKYoung
Copy link
Collaborator

ESKYoung commented May 28, 2021

Interesting! Reading the documentation, calling dotenv .env (for example) in .envrc will load the .env file. So it seems it won't do the reverse (parse the .envrc file, into a .env file), which is what the sed command does.

However, this is parsing is a bit difficult to do (see the Windows issue), and definitely not scalable (to a certain degree). I wonder instead if we should revise how we deal with environment variables.

Maybe we have a .env file with all non-secret environment variables, an untracked .secrets file in var=value format (like .env), and then direnv is optional but helps tie it all together? Although optional, you wouldn't (for example) have to load environment variables with python-dotenv using direnv. For Windows users (and PyCharm users), you'd have to load both the .env and .secrets files using python-dotenv and (for PyCharm users) the EnvFile plugin.

@exfalsoquodlibet - tagging you for reference.

Opinions welcome

@ESKYoung
Copy link
Collaborator

ESKYoung commented Jun 14, 2021

I think I'm going to go ahead and implement the above - this will be a breaking change. Main reasons are that it'll be easier for Windows users, PyCharm users, and any Unix users without direnv installed to use this repository. I'll leave direnv and .envrc in as it makes things easier for those users who do have it installed, but it'll all be optional!

Things to do (in case anyone gets there before I do!):

  1. Remove .env from .gitignore
  2. Transfer all environment variables in .envrc over to .env in var=value format
  3. Add dotenv commands in direnv to point to .env and .secrets
    • Maybe using dotenv_if_exists for .secrets or add a touch .secrets command to direnv
  4. Revise the documentation on .envrc and .secrets
  5. Update/modify the tests in tests/test_envrc.py including renaming the file
  6. Consider adding python-dotenv to requirements.txt
  7. Tag previous releases (my bad) so others can use those instead
  8. Update any references to the YouTube link to relate them to a relevant tag instead

@ESKYoung ESKYoung changed the title Improve extraction of .envrc to .env Switch environment variables to .env, and make direnv optional Jun 14, 2021
@ESKYoung ESKYoung added enhancement New feature or request question Further information is requested labels Jun 21, 2021
@ESKYoung
Copy link
Collaborator

@ESKYoung
Copy link
Collaborator

ESKYoung commented Jul 19, 2021

Rebased the above branch up to version 1.2.2, but don't have time to finish this! Any help would be much appreciated.

@AndreasThinks
Copy link

So what needs doing on this? I'm a bit lost :P

@ESKYoung
Copy link
Collaborator

ESKYoung commented Sep 28, 2021

@AndreasThinks I think all this needs are changes to the documentation, i.e.:

  • modifying any and all mentions of environment variables to refer to the VARIABLE=VALUE format
  • similarly stating those variables should go in the .env file with secrets in .secrets
  • new documentation to state this can be loaded with the python-dotenv package
  • revision of the documentation to state that using python-dotenv isn't necessary for Unix-based users with direnv installed, since that will load .env using .envrc

Might be some other bits, but should all be documentation-based.

Fair warning, might need a rebase onto the latest main first since the last commit affected documentation quite heavily!

@AndreasThinks - if you're happy to volunteer to write the docs, I can assist with the rebasing if you don't have the right privileges for this repo!

@AndreasThinks
Copy link

@ESKYoung let me go try and muck in! So is "best-practice" with the current model for windows users to load the .secret and .env files using their relative path?

I haven't done much contributing to projects on Github before and not read the contributing docs in detail... so not quite sure about how rebasing works...I was just going to do a fork with the some new docs and submit a pull request later!

@ESKYoung
Copy link
Collaborator

ESKYoung commented Oct 3, 2021

@AndreasThinks - with the changes I've suggest, yes it'll be loading it in. Relative path will be more maintainable, especially if there are collaborators in the project.

I believe @Jacobb164 has made some changes to the branch now, that may cover most (if not all) the remaining tasks! Will leave it in their capable hands! 😄

@Jacobb164
Copy link
Collaborator

Completed and merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants