Skip to content

fix(entrypoint): honor STOP_AFTER_INIT if DB already initialized #150

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix(entrypoint): honor STOP_AFTER_INIT if DB already initialized #150

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
39a064f
refactor: Extract initialization logic into separate functions for cl…
bastien8060 Sep 16, 2025
6b68776
refactor: Rearrange initialization logic for clarity in entrypoint sc…
bastien8060 Sep 16, 2025
0191113
fix(entrypoint): honor OVERPASS_STOP_AFTER_INIT when DB already initi…
bastien8060 Sep 16, 2025
409cdc0
refactor: simply init logic by consolidating exit conditions
bastien8060 Sep 17, 2025
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
44 changes: 29 additions & 15 deletions docker-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ for f in /docker-entrypoint-initdb.d/*; do
echo
done

if [[ ! -f /db/init_done ]]; then
initialize_db_dir() {
echo "No database directory. Initializing"
touch /db/changes.log
mkdir -p /db/diffs
}

setup_cookie_jar() {
Comment on lines +39 to +45
Copy link
Owner

@wiktorn wiktorn Sep 16, 2025

Choose a reason for hiding this comment

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

I do not see a lot of readability improvement by introducing those two functions and reversing the order of checks for OVERPASS_MODE and -f /db/init_done. Can you elaborate, why you think this improves readability?

Copy link
Author

@bastien8060 bastien8060 Sep 17, 2025

Choose a reason for hiding this comment

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

The refactor itself wasn’t actually "the goal". it was needed so I could introduce the logic change (moving the OVERPASS_STOP_AFTER_INIT check after the init_done guard) in a clean, isolated commit. Breaking it out into functions made the flow easier to rearrange without duplicating code, and kept the "fix commit" focused more on behavior.

Basically the fix introduced required refactors, and I split the refactors out of the fix commit, to keep the scope solely to the fix.

Copy link
Owner

@wiktorn wiktorn Sep 18, 2025

Choose a reason for hiding this comment

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

I'm not convinced that this change was necessary. What is the reason in changing:

if [[ ! -f /db/init_done ]] ; then
    if [[ "$OVERPASS_MODE" = "clone" ]]; then
        ...
    fi
    if [[ "$OVERPASS_MODE" = "init" ]]; then
        ...
    fi
fi

Into:

if [[ "$OVERPASS_MODE" = "clone" ]]; then
    if [[ ! -f /db/init_done ]] ; then
        ...
    fi
fi
if [[ "$OVERPASS_MODE" = "init" ]]; then
    if [[ ! -f /db/init_done ]] ; then
        ...
    fi
fi

if [[ "${USE_OAUTH_COOKIE_CLIENT}" = "yes" ]]; then
/app/venv/bin/python /app/bin/oauth_cookie_client.py -o /db/cookie.jar -s /secrets/oauth-settings.json --format netscape
# necessary to add newline at the end as oauth_cookie_client doesn't do that
Expand All @@ -50,8 +52,13 @@ if [[ ! -f /db/init_done ]]; then
echo "${OVERPASS_COOKIE_JAR_CONTENTS}" >>/db/cookie.jar
fi
chown -R overpass:overpass /db/cookie.jar /db/changes.log /db/diffs
}

if [[ "$OVERPASS_MODE" = "clone" ]]; then

if [[ "$OVERPASS_MODE" = "clone" ]]; then
if [[ ! -f /db/init_done ]]; then
initialize_db_dir
setup_cookie_jar
(
mkdir -p /db/db &&
/app/bin/download_clone.sh --db-dir=/db/db --source="${OVERPASS_CLONE_SOURCE}" --meta="${OVERPASS_META}" &&
Expand All @@ -63,15 +70,21 @@ if [[ ! -f /db/init_done ]]; then
echo "Failed to clone overpass repository"
exit 1
)
if [[ "${OVERPASS_STOP_AFTER_INIT}" == "false" ]]; then
echo "Overpass container ready to receive requests"
else
echo "Overpass container initialization complete. Exiting."
exit 0
fi
fi

if [[ "$OVERPASS_MODE" = "init" ]]; then
if [[ "${OVERPASS_STOP_AFTER_INIT}" == "false" ]]; then
echo "Overpass container ready to receive requests"
else
echo "Overpass container initialization complete. Exiting."
exit 0
fi
fi

if [[ "$OVERPASS_MODE" = "init" ]]; then
if [[ ! -f /db/init_done ]]; then
initialize_db_dir
setup_cookie_jar

CURL_STATUS_CODE=$(curl -L -b /db/cookie.jar -o /db/planet.osm.bz2 -w "%{http_code}" "${OVERPASS_PLANET_URL}")
# try again until it's allowed
while [ "$CURL_STATUS_CODE" = "429" ]; do
Expand Down Expand Up @@ -102,12 +115,6 @@ if [[ ! -f /db/init_done ]]; then
echo "Failed to process planet file"
exit 1
)
if [[ "${OVERPASS_STOP_AFTER_INIT}" == "false" ]]; then
echo "Overpass container ready to receive requests"
else
echo "Overpass container initialization complete. Exiting."
exit 0
fi
elif [[ $CURL_STATUS_CODE = "403" ]]; then
echo "Access denied when downloading planet file. Check your OVERPASS_PLANET_URL and OVERPASS_COOKIE_JAR_CONTENTS or USE_OAUTH_COOKIE_CLIENT"
cat /db/cookie.jar
Expand All @@ -118,6 +125,13 @@ if [[ ! -f /db/init_done ]]; then
exit 1
fi
fi

if [[ "${OVERPASS_STOP_AFTER_INIT}" == "false" ]]; then
echo "Overpass container ready to receive requests"
else
echo "Overpass container initialization complete. Exiting."
exit 0
fi
Comment on lines +124 to +129
Copy link
Owner

@wiktorn wiktorn Sep 16, 2025

Choose a reason for hiding this comment

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

Can we just move this one fi below, into the main, just before envsubst is called, and this will simplify the whole script, as we will need to check only once?

fi

# shellcheck disable=SC2016 # ignore SC2016 (variables within single quotes) as this is exactly what we want to do here
Expand Down