core icon indicating copy to clipboard operation
core copied to clipboard

Add support for conda environments

Open tsbernar opened this issue 2 years ago • 1 comments

Proposed change

This change adds support for running hass in a conda virtual environment instead of a pyenv. Before, is_virtual_env() would incorrectly return False if run from within a conda virtual environment, causing package installation to fail (from trying to run pip with the "deps" directory as the target).

Type of change

  • [ ] Dependency upgrade
  • [x] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [ ] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Checklist

  • [x] The code change is tested and works locally.
  • [x] Local tests pass. Your PR cannot be merged unless tests pass
  • [x] There is no commented out code in this PR.
  • [x] I have followed the development checklist
  • [x] I have followed the perfect PR recommendations
  • [x] The code has been formatted using Black (black --fast homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • [ ] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

tsbernar avatar Mar 19 '23 03:03 tsbernar

Hi @tsbernar

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant[bot] avatar Mar 19 '23 03:03 home-assistant[bot]

I'm not sure if we should add this, the reason is as follows: This logic is used by Home Assistant to detect its supported environments, in this case, a venv as supported and documented in our documentation.

This is adding support for Conda environments and mixing that up with that flag which has a different meaning. Just the fact you can stack environments with Conda, is probably not something we should support.

frenck avatar Mar 20 '23 09:03 frenck

Hi @frenck,

Understood on your point about supported environments. This change does make conda a supported environment, and the rest of the logic that uses this function works with a conda environment as well because conda and venv both work in the same way (by prepending a directory to your PATH).

I prefer conda to venv personally, in part because it's easier to manage different versions of python itself tied to the envs for different projects instead of just its packages, so I will keep this change in my fork to continue for personal use if not merged.

conda managing different versions of python itself is actually why the sys.prefix == sys.base_prefix check does not work here (python itself is running out of /path/to/env/bin/python[3])

Perhaps I should have also left out my comment before about the unexpected behavior in stacking, it is possible, and there are use cases for it, but I was incorrect about the stacking as the default behavior (https://docs.conda.io/projects/conda/en/latest/configuration.html) default is not to use the stacking unless the user specifies with a flag or changes the config (and they probably know what they are doing in that case).

# # auto_stack (int)
# #   Implicitly use --stack when using activate if current level of nesting
# #   (as indicated by CONDA_SHLVL environment variable) is less than or
# #   equal to specified value. 0 or false disables automatic stacking, 1 or
# #   true enables it for one level.
# # 
# auto_stack: 0

I would be happy to work on some documentation changes to add to this PR to reflect that a conda environment can be used if you're open to it. Alternatively, just keeping the code change without official support could increase compatibility without expanding the list of what needs to be officially supported (and save some future users debugging time when searching for why their dependencies are not auto-installing correctly.)

tsbernar avatar Mar 20 '23 12:03 tsbernar

We're only interested in supporting the virtual environment that is built into Python.

balloob avatar Mar 20 '23 12:03 balloob

Understood, thanks

tsbernar avatar Mar 20 '23 12:03 tsbernar