From 2fa9c5c47f8c368148c5a06dcee83f4440017307 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Thu, 29 Oct 2026 12:38:25 +0000 Subject: [PATCH] T023: the release's 15 species, the Jenkinsfile's command, the xml:base check -s now takes a list of species ids, each written to its own file, and a species named there with no events ends the run with exit 1. The Jenkinsfile passes the 15 species of release 98's published zip; whether to add Mycobacterium tuberculosis is issue #84. The stray quote that ended its docker run line is gone, and the credentials reach the shell as variables. CI's fixture job runs the release's own xml:base check (the DownloadDirectory step's grep), which passes for the fixture's release and fails for another, and its second run uses the list form. The command's shape was run from an image built from this branch against the private fixture graph, zipped as the Jenkinsfile does, and checked as the release checks. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 24 +++++++--- Jenkinsfile | 6 ++- .../server/tools/BioPAX3ExporterLauncher.kt | 48 +++++++++++++++---- specs/001-biopax-from-graph/spec.md | 2 +- specs/001-biopax-from-graph/tasks.md | 7 ++- 5 files changed, 67 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ea427bd..1f86b54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -138,14 +138,15 @@ jobs: - name: Convert each species, twice run: | docker load --input /tmp/image.tar - for run in out out-again; do - mkdir -m 777 "$run" - # Homo sapiens and Plasmodium falciparum, by their dbIds - for species in 48887 170928; do - docker run --rm --network host -v "$PWD/$run:/out" tmp-tag \ - -h localhost -b 7687 -u neo4j -p none -o /out -s "$species" - done + mkdir -m 777 out out-again + # Homo sapiens and Plasmodium falciparum, by their dbIds: one at a time, then + # both in one list as the release's command names them (T023) + for species in 48887 170928; do + docker run --rm --network host -v "$PWD/out:/out" tmp-tag \ + -h localhost -b 7687 -u neo4j -p none -o /out -s "$species" done + docker run --rm --network host -v "$PWD/out-again:/out" tmp-tag \ + -h localhost -b 7687 -u neo4j -p none -o /out -s 48887,170928 ls -l out - name: As expected @@ -175,6 +176,15 @@ jobs: fi [ ! -e one/events.owl ] || { echo "a list with an id that names no event was written"; exit 1; } + - name: The release's xml:base check (T023) + # As the release's DownloadDirectory step checks biopax.zip: every file's base + # names the release (the fixture's, 98) + run: | + files=$(find out -maxdepth 1 -name '*.owl' | wc -l) + based=$(grep -l 'xml:base="http://www.reactome.org/biopax/98' out/*.owl | wc -l) + echo "$files files, $based with the release's base" + [ "$files" -gt 0 ] && [ "$files" = "$based" ] + - name: The same bytes on a second run (FR-006) run: diff -r out out-again && echo "identical" diff --git a/Jenkinsfile b/Jenkinsfile index ff2c9eb..903647d 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -14,6 +14,10 @@ pipeline{ ECR_URL = 'public.ecr.aws/reactome/biopax3-exporter' CONT_NAME = 'biopax3_exporter_container' CONT_ROOT = '/opt/biopax3-exporter' + // The species the release publishes, by database id (spec 001 T023): human and the + // 14 it infers to, the files of release 98's biopax.zip. Mycobacterium tuberculosis + // (176806) has events of its own but is not published; whether to add it is issue #84. + SPECIES = '48887,48892,48895,48898,49591,49633,49646,56210,68320,68322,68323,68324,170928,170941,205621' } stages{ @@ -44,7 +48,7 @@ pipeline{ sh "rm -rf ${env.OUTPUT_FOLDER}/*" withCredentials([usernamePassword(credentialsId: 'neo4jUsernamePassword', passwordVariable: 'pass', usernameVariable: 'user')]){ sh """\ - docker run --network=host -v \$(pwd)/${env.OUTPUT_FOLDER}:${CONT_ROOT}/${env.OUTPUT_FOLDER} --name ${CONT_NAME} ${ECR_URL}:latest -h 127.0.0.1 -b 7687 -u $user -p $pass -o ./${env.OUTPUT_FOLDER}' + docker run --network=host -v \$(pwd)/${env.OUTPUT_FOLDER}:${CONT_ROOT}/${env.OUTPUT_FOLDER} --name ${CONT_NAME} ${ECR_URL}:latest -h 127.0.0.1 -b 7687 -u \$user -p \$pass -o ./${env.OUTPUT_FOLDER} -s ${env.SPECIES} """ dir(env.OUTPUT_FOLDER) { sh "zip -r biopax.zip ./*" diff --git a/cli/src/main/kotlin/org/reactome/server/tools/BioPAX3ExporterLauncher.kt b/cli/src/main/kotlin/org/reactome/server/tools/BioPAX3ExporterLauncher.kt index 8ae33ac..f470312 100644 --- a/cli/src/main/kotlin/org/reactome/server/tools/BioPAX3ExporterLauncher.kt +++ b/cli/src/main/kotlin/org/reactome/server/tools/BioPAX3ExporterLauncher.kt @@ -21,7 +21,7 @@ class BioPAX3ExporterLauncher { companion object { private var outputdir: String = "." private var singleId: Long = 0 - private var speciesId: Long = 0 + private var speciesIds: LongArray = LongArray(0) private var multipleIds: LongArray = LongArray(0) private var multipleEvents: LongArray = LongArray(0) private var outputStatus: Status = Status.SINGLE_PATH @@ -57,7 +57,6 @@ class BioPAX3ExporterLauncher { "toplevelpath", "The id of an event, a pathway or a reaction, written to {id}.owl", ), - FlaggedOption("species", JSAP.LONG_PARSER, "0", JSAP.NOT_REQUIRED, 's', "species", "The id of a species"), ), ) @@ -75,6 +74,22 @@ class BioPAX3ExporterLauncher { m.listSeparator = ',' jsap.registerParameter(m) + // One species or several, each to its own file: the release names the ones it + // publishes (T023) + val sp = + FlaggedOption( + "species", + JSAP.LONG_PARSER, + null, + JSAP.NOT_REQUIRED, + 's', + "species", + "A list of ids of species, each written to its own file", + ) + sp.isList = true + sp.listSeparator = ',' + jsap.registerParameter(sp) + val loe = FlaggedOption( "listevents", @@ -130,9 +145,11 @@ class BioPAX3ExporterLauncher { } Status.ALL_PATHWAYS_SPECIES -> { - val species = databaseObjectService.findByIdNoRelations(speciesId) as? Species - if (species == null) fail("$speciesId is not the identifier of a Species") - species?.let { outputPathsForSpecies(it) } + speciesIds.forEach { id -> + val species = databaseObjectService.findByIdNoRelations(id) as? Species + if (species == null) fail("$id is not the identifier of a Species") + species?.let { outputPathsForSpecies(it, named = true) } + } } Status.MULTIPLE_PATHS -> { @@ -196,14 +213,14 @@ class BioPAX3ExporterLauncher { private fun parseAdditionalArguments(config: JSAPResult) { outputdir = config.getString("outdir") singleId = config.getLong("toplevelpath") - speciesId = config.getLong("species") + speciesIds = config.getLongArray("species") multipleIds = config.getLongArray("multiple") multipleEvents = config.getLongArray("listevents") outputStatus = when { singleId != 0L -> Status.SINGLE_PATH - speciesId != 0L -> Status.ALL_PATHWAYS_SPECIES + speciesIds.isNotEmpty() -> Status.ALL_PATHWAYS_SPECIES multipleIds.isNotEmpty() -> Status.MULTIPLE_PATHS multipleEvents.isNotEmpty() -> Status.MULTIPLE_EVENTS else -> Status.ALL_PATHWAYS @@ -213,7 +230,7 @@ class BioPAX3ExporterLauncher { private fun singleArgumentSupplied(): Boolean { var count = 0 if (singleId != 0L) count++ - if (speciesId != 0L) count++ + if (speciesIds.isNotEmpty()) count++ if (multipleIds.isNotEmpty()) count++ if (multipleEvents.isNotEmpty()) count++ return count <= 1 @@ -226,12 +243,23 @@ class BioPAX3ExporterLauncher { bp.toFile(File(outputdir, "${event.dbId}.owl")) } - private fun outputPathsForSpecies(species: Species) { + // A species named with -s that has no events is an error: the release names the files + // it publishes, and one quietly missing would pass its count (T023) + private fun outputPathsForSpecies( + species: Species, + named: Boolean = false, + ) { // The species' top-level events, as the release chose them (T015) val ids = SpeciesEvents.topLevelIds(species.dbId, ReactomeGraphCore.getService(AdvancedDatabaseObjectService::class.java)) val events = SpeciesEvents.loaded(ids, ReactomeGraphCore.getService(DatabaseObjectService::class.java)) if (ids.isEmpty()) { - println("No events found for species ${species.displayName}") + if (named) { + fail( + "No events found for species ${species.displayName}", + ) + } else { + println("No events found for species ${species.displayName}") + } return } diff --git a/specs/001-biopax-from-graph/spec.md b/specs/001-biopax-from-graph/spec.md index 2fc4fa1..b099ffc 100644 --- a/specs/001-biopax-from-graph/spec.md +++ b/specs/001-biopax-from-graph/spec.md @@ -92,7 +92,7 @@ The website asks for one pathway's or one reaction's BioPAX. A library module, w ### User Story 4 - The release step runs (Priority: P2) -The release builds `biopax.zip` with this exporter in place of Pathway-Exchange's converter: the command line writes every species, the files carry the `xml:base` the release's check of the download directory requires, and the step's command runs as written. +The release builds `biopax.zip` with this exporter in place of Pathway-Exchange's converter: the command line writes the species the release names (T023: the 15 it publishes; whether to add _Mycobacterium tuberculosis_ is issue #84), the files carry the `xml:base` the release's check of the download directory requires, and the step's command runs as written. **Why this priority**: it is the switch, which comes after the proof (User Stories 1–3) and needs the release's owners to agree on its timing. diff --git a/specs/001-biopax-from-graph/tasks.md b/specs/001-biopax-from-graph/tasks.md index 155a81d..e793c7e 100644 --- a/specs/001-biopax-from-graph/tasks.md +++ b/specs/001-biopax-from-graph/tasks.md @@ -330,7 +330,12 @@ description: 'Tasks for spec 001: BioPAX Level 3 from the graph, as the release ## Phase 6: The release step (User Story 4, P2) -- [ ] T023 [US4] The Jenkinsfile's command runs as written (its stray quote, its jar, its options); it writes `biopax.zip` with one file per species, the release's species and no others (the full run of T020 wrote a sixteenth, _M. tuberculosis_, which the release step's configuration leaves out); the release's own `xml:base` check passes on it. +- [x] T023 [US4] The Jenkinsfile's command runs as written (its stray quote, its jar, its options); it writes `biopax.zip` with one file per species, the release's species and no others (the full run of T020 wrote a sixteenth, _M. tuberculosis_, which the release step's configuration leaves out); the release's own `xml:base` check passes on it. — #86, replacing #85 after its review. + - **The species:** `-s` takes a list of species ids, each written to its own file; the Jenkinsfile passes the 15 of release 98's published zip (human and the 14 it infers to), read from those files' own `xml:base`. Whether to add _M. tuberculosis_ is issue #84; if so, it is one more id there. + - **The command:** the stray quote at the end of the `docker run` line, which would have stopped the shell, is gone; the jar is the image's entry point and `-o ./output` lands in the mounted folder (the image's working directory is the container root the Jenkinsfile mounts under). + - **The release's check** is the DownloadDirectory step's (release-download-directory's Jenkinsfile): every `.owl` in `biopax.zip` must hold `xml:base="http://www.reactome.org/biopax/{release}`. CI's fixture job now runs the same grep on its output; it passes for 98 and fails for 97. + - **From #85's review:** a species named with `-s` that has no events now ends the run with exit 1, since the release's check counts files and would not miss one (shown with a species added to the private fixture graph for the run and removed after); CI's second run of the fixture job uses the list form, which its byte comparison holds to the one-by-one run; the Jenkinsfile's credentials reach the shell as `$user` and `$pass` rather than through Groovy. Noted, not changed: the release's own pattern has no `/` after the release number, so release 9 would match 98's files; the list of 15 is kept in step with what each release publishes by hand. + - **Run as written**, as far as this host allows: the image built from this branch, the command's own shape (`--network=host`, the mount, `-o ./output`, `-s` with two species the fixture graph holds) against the private fixture graph, then the Jenkinsfile's `zip -r` and the release's check on the unzipped files: two files, both with the release's base. The release server's own run, with its credentials and graph, is the release's; its time is T024. - [ ] T024 [US4] The full run's time recorded against today's step's, which the release's owners provide; the switch's timing agreed with them. ## Dependencies