Sync fork with IHTSDO/master (resolves #10)#11
Conversation
…if the previous release is not available
…order to read large result files
The aehrc fork was 232 commits behind IHTSDO/master with zero net custom changes (its 13 unique commits are self-canceling commit+revert pairs plus merge commits). This merge brings the fork up to IHTSDO/master, which includes MAINT-2526 (description term columns widened to varchar(4096) and idx_term indexes dropped), resolving #10. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Status / where we're up to — ⏸️ HOLD merge until after this week's production releaseTL;DR: the sync itself is verified clean and mergeable, but the resulting image is not a drop-in for the existing ✅ Verified on this PR (safe)
Merging the PR is low-risk in isolation — it does not touch
|
| Change | Source | Impact on rvf (templates/job.yaml, run-test.sh) |
|---|---|---|
No Dockerfile — now built via jib-maven-plugin ✔ |
RAP-93 | Image entrypoint is mainClass org.ihtsdo.rvf.App (no rvf-api.jar). The -jar rvf-api.jar line in job.yaml:60 will fail. New launch is jib's classpath form. |
| JVM flags ✔ | RAP-93 / VAL-405 | --add-opens=java.base/java.lang=ALL-UNNAMED is baked into the image (pom.xml), but any hand-rolled java invocation in job.yaml:60 must include it. AWS region is set via spring.cloud.aws.region.static=us-east-1 in the image; the AWS SDK v2 default-region chain may still want AWS_REGION/-Daws.region if cloud storage is enabled (not for a local useCloud=false test). |
| Auth model changed — SSO header, NOT Basic Auth ✔ | SecurityConfig | Sharper than first thought. New SecurityConfig wires only RequestHeaderAuthenticationDecorator (IHTSDO SSO header auth) with anyRequest().authenticated(); no httpBasic. run-test.sh:7-8 Basic-Auth (RVF_USER/RVF_PASSWORD) will get a JSON 401 regardless of spring.security.user.*. Fix requires either re-adding httpBasic() (fork patch to SecurityConfig) or supplying the SSO header — setting matching env vars alone is not sufficient. |
| Module Storage Coordinator ✔ | VAL-405 / VAL-493 | New module.storage.* props. Defaults are sane for a quick test (useCloud=false, local.path=store/local/, application.properties:56-62) — should work without cloud config. |
rvf.empty-release-file=empty-rf2-snapshot.zip ✔ |
RAP-13 | Confirmed at application.properties:50. Used as placeholder when no previous release is supplied; must ship in image or be mounted. |
| MySQL 8 | PIP-521 | rvf already on mysql:8.0.28 (job.yaml:41) — no change. |
fix-long-terms.sh |
local | Now redundant (schema accepts 4096, so the length >= 255 awk path never trips). Harmless to leave for first test; delete in a cleanup commit. |
🧪 Building a test image locally (notes for whoever picks this up)
Target: ontoserver.azurecr.io/aehrc-rvf/release-validation-framework:sync-ihtsdo (non-:latest). Three local Windows build-env gotchas found:
checkout-resources.shfails on Windows —.gitattributes* text=autogives it CRLF endings, breaking bash. (Linux CI is unaffected.)- The script shells out to a nested
mvn, which needsJAVA_HOME(onlyjavaon PATH locally). az(Azure CLI) not installed — needed to auth the ACR push.
Workaround that sidesteps 1 & 2: pre-clone the two resource repos (snomed-drools-rules, snomed-release-validation-assertions @ master) and build with -Dexec.skip=true. To push only the :sync-ihtsdo tag (and not also :8.3.1, which the pom's <tags> block would otherwise push and could overwrite a release tag in the registry):
mvn clean package jib:build -DskipTests -Pdocker-amd64 -Dexec.skip=true \
-Dimage=ontoserver.azurecr.io/aehrc-rvf/release-validation-framework:sync-ihtsdo \
-Djib.to.tags=
Next steps
- Hold this PR until after the production release.
- Branch
rvfand adapt:job.yamlentrypoint → jib launch form; resolve the auth model change (decide: re-addhttpBasicupstream-side, or switch the test harness to SSO header); confirm module-storage + empty-release defaults. - Build the
:sync-ihtsdotest image, point thervfbranch at it, run the pipeline end-to-end. - Only then merge this PR and roll the image forward. Clean up
fix-long-terms.shseparately.
See also: See also : https://github.com/aehrc/rvf/issues/25
Summary
Brings
aehrc/masterup to date withIHTSDO/release-validation-framework@master, which it had fallen ~232 commits / ~2 years behind.The fork had diverged but with zero net custom content — its 13 unique commits are entirely self-canceling: four
commit+revertpairs (M1 build helpers, an RC test-run script, "pre-requisite SQL" support, and a varchar widening attempt4d28428reverted by82c1ba0) plus three older IHTSDO merge commits. Themerge_base...aehrc/masternet file diff was empty, so this merge applied with no conflicts and the resulting tree is identical to IHTSDO/master.Resolves #10
Issue #10 asked to widen the description
termcolumn so full-length SNOMED terms aren't truncated. IHTSDO already shipped this in MAINT-2526 (e0ad400):description_f,description_s,description_dtermcolumns are nowvarchar(4096) collate utf8_bin(wasvarchar(256));idx_term(term)indexes were dropped — required because all tables are MyISAM (1000-byte index-key cap;varchar(4096)utf8 = 12,288 bytes would failCREATE TABLE). This is exactly why the fork's earlier widen-only attempt (4d28428) was reverted.Syncing therefore resolves the issue the correct way (widen and drop the index), without re-introducing the original failure.
Closes #10.Verification
git diff origin/master --stat(origin = IHTSDO) is empty — synced tree matches IHTSDO/master exactly.termcolumns insrc/main/resources/sql/create-tables-mysql.sqlarevarchar(4096) collate utf8_bin; noidx_termremains.Notes / follow-ups
aehrc/rvfcleanup (per Widen description term column to support full-length SNOMED terms (varchar 4100) #10): bump the RVF image tag, deletetestscripts/fix-long-terms.sh, and remove its invocation intestscripts/run-test.sh.🤖 Generated with Claude Code