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

fix(filebrowser): configure before running #400

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions filebrowser/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fi

printf "🥳 Installation complete! \n\n"

printf "👷 Starting filebrowser in background... \n\n"
printf "🛠️ Configuring filebrowser \n\n"

ROOT_DIR=${FOLDER}
ROOT_DIR=$${ROOT_DIR/\~/$HOME}
Expand All @@ -21,10 +21,21 @@ if [ "${DB_PATH}" != "filebrowser.db" ]; then
DB_FLAG=" -d ${DB_PATH}"
fi

# Check if filebrowser db exists
if [ ! -f ${DB_PATH} ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [ ! -f ${DB_PATH} ]; then
if [ -n "$DB_FLAG" ]; then

Ideally I'd want DB_FLAG to be an array, so it can handle spaces in DB_PATH, but for now let's at least improve this check to avoid syntax errors.

filebrowser $DB_FLAG config init >> ${LOG_PATH} 2>&1
filebrowser $DB_FLAG users add admin "" --perm.admin=true --viewMode=mosaic >> ${LOG_PATH} 2>&1
fi

filebrowser $DB_FLAG config set --baseurl=${SERVER_BASE_PATH} --port=${PORT} --auth.method=noauth --root=$ROOT_DIR >> ${LOG_PATH} 2>&1
Comment on lines +26 to +30
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
filebrowser $DB_FLAG config init >> ${LOG_PATH} 2>&1
filebrowser $DB_FLAG users add admin "" --perm.admin=true --viewMode=mosaic >> ${LOG_PATH} 2>&1
fi
filebrowser $DB_FLAG config set --baseurl=${SERVER_BASE_PATH} --port=${PORT} --auth.method=noauth --root=$ROOT_DIR >> ${LOG_PATH} 2>&1
filebrowser $DB_FLAG config init 2>&1 | tee -a "${LOG_PATH}"
filebrowser $DB_FLAG users add admin "" --perm.admin=true --viewMode=mosaic 2>&1 | tee -a "${LOG_PATH}"
fi
filebrowser $DB_FLAG config set --baseurl=${SERVER_BASE_PATH} --port=${PORT} --auth.method=noauth --root=$ROOT_DIR 2>&1 | tee -a "${LOG_PATH}"

Just a suggestion, this will allow showing the output in the Coder UI as well as the log file (useful in case of error).


printf "👷 Starting filebrowser in background... \n\n"


printf "📂 Serving $${ROOT_DIR} at http://localhost:${PORT} \n\n"

printf "Running 'filebrowser --noauth --root $ROOT_DIR --port ${PORT}$${DB_FLAG} --baseurl ${SERVER_BASE_PATH}' \n\n"
printf "Running 'filebrowser $DB_FLAG' \n\n"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
printf "Running 'filebrowser $DB_FLAG' \n\n"
printf "Running 'filebrowser %s'\n\n" "$DB_FLAG"

Since we're using printf, this is the typical way to include variable content. 😄


filebrowser --noauth --root $ROOT_DIR --port ${PORT}$${DB_FLAG} --baseurl ${SERVER_BASE_PATH} > ${LOG_PATH} 2>&1 &
filebrowser $DB_FLAG >> ${LOG_PATH} 2>&1 &
Copy link
Member

Choose a reason for hiding this comment

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

I want to make it clear that this issue was not introduced by this PR.

But I'll just mention this again; this will break if DB_PATH has spaces in it. A better way would be to initialize this as an array and use it in a way that handles spaces:

args=()
if [ "${DB_PATH}" != "filebrowser.db" ]; then
	args+=(-d "${DB_PATH}")
fi

Then use this as:

filebrowser "${args[@]}"

(I did not verify if $$ vs $ is required here due to terraform templates.)

This syntax works even in bash-3.2, so should be fine to use. (I opted for arrays as I see we already have a heavy dependency on Bash here.)


printf "📝 Logs at ${LOG_PATH} \n\n"
Loading