-
Notifications
You must be signed in to change notification settings - Fork 4
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
build: Provide setup script for running as a systemd service #35
Conversation
scripts/install-service.sh
Outdated
../resources/poglink.service \ | ||
${OUTPUT_PATH} | ||
|
||
# TODO: Copy sample config to ~/.poglink if it doesn't exist yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will circle back to this before merging; Otherwise it would need to be abundantly obvious to the user that they need to create/populate this file.
scripts/install-service.sh
Outdated
# Check whether virtual environment is active | ||
if [ -z "${VIRTUAL_ENV}" ] && [ "${FORCE}" != "true" ]; then | ||
echo "Warning: Virtual environment not detected! It is highly recommended to install this program to run under a virtual environment. Please activate your virtual environment and run this script again." | ||
echo "If you're certain you'd like to continue without a virtual environment, you can run again with 'FORCE=true'." | ||
exit 1 | ||
fi | ||
|
||
# Install poglink python package | ||
pip install poglink | ||
|
||
# Obtain path to python executable | ||
if [ -z "${VIRTUAL_ENV}" ]; then | ||
PYTHON_EXECUTABLE="$(which python3)" | ||
else | ||
PYTHON_EXECUTABLE="${VIRTUAL_ENV}/bin/python3" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I came up with to help encourage usage within a virtual environment, while not explicitly preventing someone from ignoring that advice.
I use pyenv
and it should work equally well with venv
. Please do test if you'd like though.
a59603b
to
eb33506
Compare
cd "$(dirname "$0")" | ||
|
||
# Check whether virtual environment is active | ||
if [ -z "${VIRTUAL_ENV}" ] && [ "${FORCE}" != "true" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If FORCE=true
then this block will pass and it will install poglink
on the system Python environment. This might be desireable if using a dedicated VM for this bot and didn't want to bother with a venv
when you knew there was nothing to worry about conflicting with.
fi | ||
|
||
# Install poglink python package | ||
pip install poglink |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't want to pin a specific version or force an update; I feel like that should be up to the user to decide if they want a specific version or want to update.
sudo install \ | ||
-v \ | ||
--backup \ | ||
--suffix .bak \ | ||
../resources/poglink.service \ | ||
${SERVICE_FILE_PATH} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like to use install
instead of cp
here because it allows me to set a backup option, in case you run accidentally and overwrite a service file you've modified.
sudo sed -i "s@\#\#PYTHON\#\#@${PYTHON_EXECUTABLE}@g" "${SERVICE_FILE_PATH}" | ||
sudo sed -i "s@\#\#DATA_DIR\#\#@${DATA_DIR}@g" "${SERVICE_FILE_PATH}" | ||
sudo sed -i "s@\#\#USER\#\#@${USER}@g" "${SERVICE_FILE_PATH}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a simple search and replace for the delimited tokens I've defined.
if [ ! -e ${DATA_DIR}/config.yaml ]; then | ||
echo "Poglink config not found in $DATA_DIR; Using creating from sample..." | ||
mkdir -p ${DATA_DIR} | ||
cp ../sample-config.yaml ${DATA_DIR}/config.yaml | ||
|
||
echo -e "\n\nIMPORTANT!!! A sample configuration file has been created at ${DATA_DIR}/config.yaml . Please edit this file before starting the application for the first time." | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leave existing config files alone if they are already found in the data directory.
71cf726
to
1342f30
Compare
1342f30
to
d9c99b9
Compare
Housekeeping reminder @FM-17 |
d9c99b9
to
23c4984
Compare
Codecov Report
@@ Coverage Diff @@
## main #35 +/- ##
=======================================
Coverage 73.13% 73.13%
=======================================
Files 13 13
Lines 510 510
=======================================
Hits 373 373
Misses 137 137 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally got around to reviewing this. It looks great. I'd say we're good to merge pending this follow-up, and any other changes that may be needed to bring this up to date.
Housekeeping reminder @travipross |
Elaboarating on the housekeeping reminder: Want to circle back to this before merging. |
🎉 This PR is included in version 0.9.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
I've created a script which I think will walk the user through the process of installing as a service. I've tested a few times in several variations on my machine and it seems to work great, so I think it's good to include.