fix(entrypoint): honor STOP_AFTER_INIT if DB already initialized #150
Conversation
…alized STOP_AFTER_INIT was only applied if init/clone actually created the DB. When /db/init_done already existed, containers in init mode ignored the flag and stayed running. Now the STOP_AFTER_INIT check is evaluated after the init_done guard, so containers exit consistently in init/clone modes regardless of whether the database was freshly created or already present.
|
Change is deliberately minimal and refactors made step by step leading to the change (for clarity of the review). Let me know what you think! |
|
it just makes the behaviour (way the flags are ran) consistent, because it's not very useful (if not counterintuitive) if it were to only exit after init (which would often be done manually). Setting mode=init will always run init (if database doesn't exist), and it will always exit early if told to exit afterwards. |
|
Since this change changes behaviour in edge-cases (even though it doesn't break normal usage), it might be worth starting to versioning and changelogs. For example, bumping a minor version: Also, branches and tags (on both Github and Docker Hub) can be used for versioning. |
| if [[ "${OVERPASS_STOP_AFTER_INIT}" == "false" ]]; then | ||
| echo "Overpass container ready to receive requests" | ||
| else | ||
| echo "Overpass container initialization complete. Exiting." | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
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?
| initialize_db_dir() { | ||
| echo "No database directory. Initializing" | ||
| touch /db/changes.log | ||
| mkdir -p /db/diffs | ||
| } | ||
|
|
||
| setup_cookie_jar() { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
| fi | ||
| fi | ||
|
|
||
| if [[ "$OVERPASS_MODE" = "init" || "$OVERPASS_MODE" = "clone" ]]; then |
There was a problem hiding this comment.
What is the point of this check? Since only "init" or "clone" are allowed values this if is always true.
Description:
This PR ensures
OVERPASS_STOP_AFTER_INITis applied consistently in both init and clone modes.Before, the flag was only checked when the database was freshly created. If /db/init_done already existed, the container ignored the flag and continued running, which broke automation workflows relying on idempotent init steps. Common Dev/Ops patterns separates the initialization and the serving stages.
Changes in this PR:
Result: containers in init or clone mode now exit consistently when OVERPASS_STOP_AFTER_INIT=true, regardless of whether initialization work was performed.