Skip to content

Make the deploy environment contract explicit: one home for SIDEWALK_PANO_DIR, a current template, and a preflight check #4960

Description

@jonfroehlich

Summary

Follow-up to #4925 and #4926. The app now refuses to boot when a persistent media directory is unset or points
somewhere a deploy will delete it — which is the behaviour we want, but it turns the deploy environment into a hard
startup dependency. Right now that environment is only partly written down, and a variable that goes missing
overrides the app's default instead of falling back to it.

This ticket is the ops-side counterpart to the app changes already merged: make the launcher's environment contract
explicit, and make a missing variable say so by name instead of taking a stage down with an indirect error.

Three small changes, all in the private ops repo. None of them change behaviour on a correctly configured stage.

Background: why a missing variable is worse than it looks

The per-city launcher passes environment through as export FOO=$FOO. On an unset variable that exports the
empty string rather than nothing — and Play's HOCON ${?VAR} substitution guards unset, not empty.
Verified directly against config-1.4.5:

$ SIDEWALK_PANO_DIR= jshell → d = ".panos"; d = ${?SIDEWALK_PANO_DIR}
[]

So the empty value wins over the default the app ships. Before #4927 that meant silent misbehaviour — the
.story-media data loss in #4925 is exactly this shape. After #4927 it means a stage refuses to start, which is
the better failure, but it makes every undocumented variable a thing that can stop a deploy.

1. SIDEWALK_PANO_DIR belongs with the other tracked config

pano.images.directory / SIDEWALK_PANO_DIR is the one media directory the launcher exports that has no
definition anywhere in the ops repo — it's set per-server rather than in tracked config. That means a stage stood up
from the repo alone would not have it, and it's one of the directories #4927's boot check treats as fatal, so that
stage would not start.

It doesn't belong in the calculated config: unlike the other three media dirs it isn't composed from other
variables and isn't per-stage. The natural home is the tracked server-defaults config, which already carries
host-specific absolute paths — the Java, Python, and Postgres install paths and the deployment root are all equally
specific to the machine we run on. Putting it there also gives the local-override config something to override,
which is the mechanism that layer exists for.

Ask: move it to the tracked server config with its current value, and delete the per-server copy so there's one
source of truth. Worth taking the value from what's currently set rather than from anything written down — the
tracked file is read last and would win, so a wrong path would quietly stop the app finding stored panoramas.

2. The private-config template no longer matches what the launcher reads

The template that documents "the values a new stage needs" has drifted well behind the launcher. Counted against
what the launcher actually exports, twelve variables it needs are absent from the template, including both
Silhouette keys, the application secret, all six Infra3D client values, the Mapillary token, the Gemini key, and
SIDEWALK_PANO_DIR.

That matters mostly for disaster recovery and for standing up a new host: the template is the only written record
of the contract, and following it today produces a stage that cannot sign a session or reach three imagery
providers.

Ask: regenerate the template from the launcher's actual variable list, so the two can't silently diverge again.
Ideally this becomes mechanical — a check that every variable the launcher exports appears in the template — rather
than something that has to be remembered.

3. A preflight check, so a missing variable names itself

The failure mode above is only unpleasant because it surfaces late and indirectly: the app starts, resolves an
empty path, and throws. A loop in the launcher that checks every required variable is non-empty before starting the
JVM would turn that into one line naming exactly what's missing.

Ask: a preflight that fails with the list of empty variables before launch. This is the change that prevents the
next instance of #4925 generically, rather than one variable at a time.

While that's being touched, two related things in the same scripts:

  • The guard clauses use the pattern >&2 echo "..." && exit. Bare exit returns the status of the last command —
    the echo — so those refusal paths exit 0, and a caller can't distinguish "refused" from "did the work".
    Giving them a non-zero status makes the guards actually guard.
  • In the build script, the dependency-install and asset-build steps are sequential statements with no status
    checks, so only the final step determines the exit code. A failed frontend asset build currently still ships, and
    cities restart onto stale bundles. Chaining the steps would catch it.

Why this is worth doing now rather than later

#4925 was not a code bug — the upload, the DB write, and the serving were all correct. It lived in the seam between
our config defaults and a deploy environment in a different repo, and neither side knew about the other's half of
the contract. We've now fixed our half: the app validates its media directories at boot and fails loudly. The three
items above are the other half — making the environment self-describing, so the contract is checkable from both
ends instead of held in memory.

Related

🤖 Generated with Claude Code (claude-opus-5[1m])

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority: MediumMedium urgency.Server ManagementProduction server ops, deployment, and infrastructure.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions