fix: silent-failure fixes from the acdc deployment (version normalisation, idempotent indexd, --allow-prod, commons URL) - #1
Open
JoshuaHarris391 wants to merge 4 commits into
Conversation
…lidate semver) The uploader stores versions v-stripped (group(1) of ^v?(x.y.z)$) while the delete path interpolated the operator's string literally into the Athena predicate, so --version v1.5.4 matched zero rows; the bulk wrapper's exit-3-as-skipped semantics then reported the run as clean. Normalise v-prefixed input and reject anything that is not three-part semver or 'all' — the column can only hold x.y.z, so no valid input is refused.
Re-submitting a file creates a NEW indexd revision with a new did every time (baseid never overwrites), so an unfiltered re-run duplicated the registry — measured live as 46,598 rows for 23,295 unique files. Before registering, read the (file_name, md5) pairs already recorded for the study at the SAME indexd endpoint (staging must never mask prod) and skip unchanged files; a changed md5 still registers, which is exactly when a new revision is wanted. A missing registry table is a first run, not an error. --force restores the old behaviour, and the misleading 'idempotent at the API level' docstring is corrected.
Bulk upload to production was previously impossible (the wrapped script hard-aborted on any 'prod' with no escape hatch) while the CLI itself performed no check at all. Now: refuse with exit 2 unless --allow-prod, then require the env name typed locally before any EC2 dispatch (SSM has no TTY). The gate also tests the resolved study keys, since --env staging with --studies ausdiab_prod is a production write. The remote re-entry carries a hidden --prod-confirmed marker instead of re-prompting into a TTY-less session. Dead abort_if_prod removed.
…t-less The bare commons URL was derived inline and immediately suffixed with /api/<version>, so nothing else could reuse it — and nothing recorded why Gen3Auth must never receive an explicit endpoint: gen3.auth falls back to the Workspace Token Service when a passed endpoint disagrees with the credential's iss, and WTS is not deployed on these commons, so that path fails with a misleading 502 on /wts/external_oidc/. The extracted function carries the rationale; the environment selects the credential and the credential selects the commons.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports four battle-tested fixes from the legacy acdc deployment's CLI, each closing a failure mode hit live:
delete metadata—--version v1.5.4matched zero rows (the uploader stores versions v-stripped) and the bulk wrapper reported the run as a clean skip. Now normalised; non-semver rejected with exit 2.--forcerestores the old behaviour; changed md5 still re-registers.--allow-prodformetadata upload-all— bulk prod upload was impossible (hard bash abort, no escape) with no local guard. Now: refuse with exit 2 unless--allow-prod, typed env confirmation locally before dispatch, gate also checks resolved study keys.commons_url_from_jwtextracted — records structurally why Gen3Auth is never given an endpoint (the WTS 502 trap).Each commit carries its own rationale; tests follow the repo's docstring style. 308 tests pass.