Skip to content

Update systemd unit file for virtualenv #1785

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

Merged
merged 1 commit into from
Jan 17, 2017

Conversation

asbach
Copy link
Contributor

@asbach asbach commented Jan 15, 2017

Description:
The current systemd unit file will provoke an error because the 'ExecPre' line calls a shell built-in function and all systemd Execs need to use an absolute path. The proposed change sets the python environment using the provided 'Environment' calls as used by systemd.

The current systemd unit file will provoke an error because the 'ExecPre' line calls a shell built-in function and all systemd Execs need to use an absolute path. the proposed change sets the python environment using the provided 'Environment' calls as used by systemd.
@mention-bot
Copy link

@asbach, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tmcneal, @balloob and @mmaret-geny to be potential reviewers.

Copy link
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🐦

@fabaff fabaff merged commit c1aeb7e into home-assistant:current Jan 17, 2017
@WeeBull
Copy link

WeeBull commented Jan 17, 2017

These environment lines aren't necessary I believe. Any python executables installed inside a virtualenv have their python executable specifically defined.

For example, my hass executable looks like this:

#!/srv/homeassistant/homeassistant_venv/bin/python3

# -*- coding: utf-8 -*-
import re
import sys

from homeassistant.__main__ import main

if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

Note the hashbang line.

@andrey-git
Copy link
Contributor

@asbach This doesn't work
Using $ in Environment is not allowed.

The resulting path is "$VIRTUAL_ENV/bin:$PATH" (literally)

@asbach
Copy link
Contributor Author

asbach commented Jan 30, 2017

@andrey-git I just reread the docs and as you already noticed, unit files do not support variable expansion in the Environment= directive.
Are there any objections to using the style as suggested by @WeeBull?

@andrey-git
Copy link
Contributor

I don't understand his proposal.

For me I just replaced "$VIRTUAL_ENV/bin:$PATH" by manually expanded values.

@asbach
Copy link
Contributor Author

asbach commented Jan 30, 2017

Because the virtualenv already creates binaries with a hashbang pointing to the correct binary of Python. Setting the PATH is most likely not required. Maybe one of the main developers can jump in? Does home assistant depend on a correct PATH in relation to the virtual environment binaries?

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.

5 participants