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

Load .env AFTER environment activation #3299

Closed
wants to merge 5 commits into from
Closed

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Nov 25, 2018

This makes the environment's PATH and VIRTUAL_ENV variables available for use in the .env file. This is helpful for configuring tools like mypy that are not naturally aware of virtual environments.

I got this problem when trying to configure mypy, which is not aware of virtual environments, and it is recommended to set MYPYPATH to the environment’s site-packages. It is, however, quite awkward to do without VIRTUAL_ENV being available, especially if PIPENV_VENV_IN_PROJECT is not set.

Virtual environment activation (for both virtualenv and venv) changes two environment variables: VIRTUAL_ENV and PATH (to prepend the scripts/bin directory). The latter could be a source of backward-incompatibilities, if the user also prepends to PATH in .env. This would likely be uncommon IMO.

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix, .feature, .behavior, .doc. .vendor. or .trivial (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

This makes the environment's PATH and VIRTUAL_ENV variables available for
use in the .env file. This is helpful for configuring tools like mypy that
are not naturally aware of virtual environments.
@techalchemy
Copy link
Member

I’m like 90% sure the precise moment when this gets loaded is super important for other reasons. Its less about what the activation alters and more about what the user did in .env to start. If as I believe some users do, they make changes to make the activation itself work, this will break

@techalchemy
Copy link
Member

/cc @nateprewitt I think you were involved in the early implementation of this, do you have any recollection about the specifics on that? I looked through the commit history / issues and I couldn't see anything relevant...

@uranusjr
Copy link
Member Author

I don’t think the fact that it is done like this supports the claim that there is a reason behind it. If it’s that important, it would be very possible someone would have put a comment in there. By extension, therefore, I am inclined to think that since there is no comments or documentation on this, the current order of operations is merely happenstance. I would be glad to discuss if there is evidence, of course. (I am not saying you’re wrong; I just don’t put trust in memories in general.)

@techalchemy
Copy link
Member

I don’t actually remember it being important just that order of operations is. Absent any indicators that this will make a difference I’m good with it, though for your specific use case I wound up just adding an invoke task task to set the myrun mypy using pipenv run inv typecheck. The alternative is to just set it in setup.cfg

@matteius
Copy link
Member

I had done some work earlier this year that seems to supersede this around some issue reports, that change made us load the .env earlier in the process, so I don't think this PR is going to be relevant anymore.

@matteius matteius closed this Aug 13, 2022
@uranusjr uranusjr deleted the activate-before-load branch August 14, 2022 04:49
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.

3 participants