Wars sharedclasspath#617
Conversation
📝 WalkthroughWalkthroughThis PR introduces a complete multiwar deployment system that packages 21+ GridSuite microservices as WARs on a single Tomcat container. It includes a build automation script that generates lightweight wrapper projects, manages shared dependencies, applies selective auto-configuration exclusions, Docker Compose orchestration, and comprehensive documentation of classloading challenges and solutions. ChangesMultiwar Deployment Architecture
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
docker-compose/multiwar/docker-compose.multiwar.yml (2)
53-152: ⚖️ Poor tradeoffHeavy duplication across non-WAR services.
config-server,config-notification-server,study-notification-server,directory-notification-server, andgatewayshare identicalJAVA_TOOL_OPTIONS,command,sysctls,memswap_limit, anddeploy.resources.limitsblocks. The inline comment at line 55-57 already notes this should reuse definitions fromdocker-compose.base.yml. A YAML anchor (&svc-defaults/<<: *svc-defaults) would also work locally if base reuse is out of scope for the POC.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/docker-compose.multiwar.yml` around lines 53 - 152, Extract the repeated common service configuration (JAVA_TOOL_OPTIONS, command, sysctls, memswap_limit, deploy.resources.limits) into a YAML anchor (e.g. &svc-defaults) placed near the top of the services block, then reference it in each service (config-server, config-notification-server, study-notification-server, directory-notification-server, gateway) using the merge key (<<: *svc-defaults); ensure the anchor contains the exact keys to be shared and keep service-specific keys like image, ports, volumes and any extra mounts outside the merged block so each service still overrides or extends as needed.
6-10: 💤 Low valuePin the Tomcat image by digest for reproducibility.
tomcat:10.1-jdk21is a floating tag — the underlying image will shift over time, which can silently change JDK build, OS packages, or default config and break the carefully-tuned classloading setup.apps-metadata-serverat line 246 already pins by digest; same treatment here would make this POC reproducible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/docker-compose.multiwar.yml` around lines 6 - 10, The tomcat service currently uses a floating tag "image: tomcat:10.1-jdk21" which must be pinned by digest for reproducibility; locate the tomcat service block (the "tomcat" service and the "image: tomcat:10.1-jdk21" line) and replace the tag with the corresponding digest-pinned reference (e.g., "tomcat@sha256:<digest>") obtained from the Docker registry (docker pull or manifest inspect) so the docker-compose.multiwar.yml uses an immutable image reference.docker-compose/multiwar/wars.sh (3)
496-509: 💤 Low valueDead variables flagged by shellcheck.
- Line 496:
t(the type field) is read but never used in the loop body — rename to_for clarity.- Line 530:
SHARED_DEPS_FILEis assigned and never read anywhere else in the script.- Line 783 (
${propfile#$res_dir/}): the pattern variable is unquoted, so glob metachars in$res_dir(unlikely but possible) would be interpreted; quote it.🔧 Proposed cleanups
- while IFS=: read -r g a t classifier v; do + while IFS=: read -r g a _ classifier v; do- # Export for use by generate_wrappers - SHARED_DEPS_FILE="$deps_dir/_shared.txt" DEPS_DIR="$deps_dir"- local rel="${propfile#$res_dir/}" + local rel="${propfile#"$res_dir"/}"Also applies to: 530-531
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/wars.sh` around lines 496 - 509, The loop reading dependencies uses an unused variable `t` and should be renamed to `_` in the while-read that constructs `shared_deps_xml` (look for the while IFS=: read -r g a t classifier v; do ... done < "$deps_dir/_shared.txt") to silence shellcheck; remove or stop assigning the unused `SHARED_DEPS_FILE` variable (search for its assignment around the 530 area) since it’s never used; and quote the parameter expansion `${propfile#$res_dir/}` (used later where propfile is stripped of $res_dir) to prevent accidental globbing by changing to a quoted form so the pattern is treated literally.
451-453: 💤 Low value
grep -P(PCRE) is not portable.Lines 451 uses
grep -oP, which depends on PCRE-enabled grep. BSD/macOSgrepdoes not support-Pby default and fails with "invalid option". If this script is only ever run on a Linux build host that's fine, otherwise preferawkorsed.- grep -oP '^\s+\S+' "$deps_file" | sed 's/^[[:space:]]*//' | \ + awk 'NF && /^[[:space:]]+\S+/ {sub(/^[[:space:]]*/, ""); print $1}' "$deps_file" | \ awk -F: 'NF==5 {print $1":"$2":"$3"::"$4} NF>=6 {print $1":"$2":"$3":"$4":"$5}' | \ sort -u > "$deps_dir/${ctx}.parsed"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/wars.sh` around lines 451 - 453, The use of grep -oP is not portable; replace the grep+sed step with a POSIX-compatible awk that extracts the first non-space token from each line of "$deps_file" before piping to the existing awk/ sort sequence. For example, change the pipeline that starts with grep -oP '^\s+\S+' "$deps_file" to use awk '{ if (match($0,/^[[:space:]]*[^[:space:]]+/)) print substr($0,RSTART,RLENGTH) }' "$deps_file" | awk -F: 'NF==5 {print $1":"$2":"$3"::"$4} NF>=6 {print $1":"$2":"$3":"$4":"$5}' | sort -u > "$deps_dir/${ctx}.parsed", keeping the same variables deps_file, deps_dir and ctx so the rest of wars.sh remains unchanged.
285-370: 💤 Low valueImplicit dependency on
DEPS_DIRset byfind_common_deps.
add_shared_lib_autoconfig_exclusionsreads${DEPS_DIR}/${ctx}.parsed(line 289) butDEPS_DIRis only assigned at the end offind_common_deps(line 531) and never declared at module scope. Withset -u, calling these functions out of order — or in isolation while debugging — will fail with an "unbound variable" error far from the cause. Consider initializingDEPS_DIRnext to the other path globals at the top of the script.SHARED_LIB_DIR="$SCRIPT_DIR/gen/shared-lib" +DEPS_DIR="$SCRIPT_DIR/gen/deps"…and then drop the assignment at line 531.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/wars.sh` around lines 285 - 370, The function add_shared_lib_autoconfig_exclusions relies on DEPS_DIR but DEPS_DIR is only set inside find_common_deps, causing unbound-variable failures when functions are invoked out of order; to fix, declare and initialize DEPS_DIR alongside the other top-level path globals at the top of the script (so DEPS_DIR exists even before find_common_deps runs) and then remove the later assignment to DEPS_DIR from find_common_deps; update references to DEPS_DIR in add_shared_lib_autoconfig_exclusions and find_common_deps as needed to ensure the single authoritative initialization is at the module scope.docker-compose/multiwar/README.md (3)
3-3: ⚡ Quick winRemove AI generation attribution from user-facing documentation.
This attribution is unprofessional in production documentation. Users care about accuracy and utility, not the generation method. If attribution is necessary, place it in commit messages or internal comments instead.
📝 Proposed fix
# WAR Deploy Tool -Generated with little supervision by opus4.6. - Builds GridSuite microservices as WAR files for docker-compose deployment, **without modifying any original source files**.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/README.md` at line 3, Remove the user-facing AI generation attribution line "Generated with little supervision by opus4.6." from the README content so the documentation no longer references the generation method; if you need to retain provenance, add that note to the commit message or an internal comment in the repo instead.
1-59: ⚡ Quick winDocument architectural trade-offs and limitations.
This multiwar approach eliminates the isolation benefits of microservices architecture. Consider adding a "Limitations & Trade-offs" section that covers:
- Shared classloader risks (dependency conflicts, security boundaries)
- Single point of failure (one Tomcat crash affects all services)
- No independent scaling
- Resource contention
- Debugging complexity
The AI summary references
SHARED_CLASSLOADING_ISSUES.txtwhich likely documents some concerns—consider summarizing key points here for visibility.📋 Suggested section to add
## ⚠️ Limitations & Considerations This is a proof-of-concept deployment strategy with significant trade-offs: - **No service isolation**: All services share the same JVM, classloader, and resources - **Single point of failure**: If Tomcat crashes, all services go down - **Dependency conflicts**: Shared classpath can cause version conflicts (see SHARED_CLASSLOADING_ISSUES.txt) - **No independent scaling**: Cannot scale individual services based on load - **Security boundaries**: Services share the same security context **Use case**: Development/testing environments where resource efficiency is prioritized over production-grade isolation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/README.md` around lines 1 - 59, Add a new "Limitations & Trade-offs" section to docker-compose/multiwar/README.md that concisely documents architectural drawbacks of the multi-WAR approach: mention shared classloader risks (dependency conflicts, refer to SHARED_CLASSLOADING_ISSUES.txt), single point of failure (one Tomcat/JVM for all services), no independent scaling or per-service resource limits, increased debugging complexity and weakened security boundaries, and recommend this strategy only for dev/testing; also note how to enable/disable services via the MANIFEST array in war-deploy.sh for operational context.
14-24: 💤 Low valueAdd language identifier to fenced code block.
The ASCII diagram should specify a language identifier for proper rendering. Use
textfor ASCII art.📝 Proposed fix
-``` +```text Original server (unmodified) Wrapper project (generated) ┌─────────────────────────┐ ┌─────────────────────────┐As per coding guidelines, the static analysis tool markdownlint-cli2 flagged: "Fenced code blocks should have a language specified (MD040)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose/multiwar/README.md` around lines 14 - 24, The fenced ASCII diagram block in the README lacks a language identifier; update the diagram's code fence to use the text language by changing the opening fence from ``` to ```text (and keep the closing fence ```), so the ASCII art (the "Original server (unmodified) ... report-server.war" diagram) is rendered correctly and satisfies MD040.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose/multiwar/docker-compose.multiwar.yml`:
- Around line 12-13: The comment incorrectly states that the mounted server.xml
configures Host startStopThreads="10" for parallel WAR deployment; update the
comment to reflect the actual configuration and reasoning by noting that
docker-compose/multiwar/server.xml sets startStopThreads="1" (sequential) to
avoid Logback ConcurrentModificationException described in
SHARED_CLASSLOADING_ISSUES.txt and implemented in wars.sh; change the comment
above the volume mount to accurately describe that server.xml enforces
sequential deployment via startStopThreads="1" and reference the wars.sh
behavior and ISSUE 4 rationale so future readers understand the intentional
sequential setting.
- Around line 38-41: The entrypoint's Elasticsearch wait loop (the entrypoint
array that runs curl and then catalina.sh run) can hang forever; change it to
bound the wait with a max retry count or a timeout so the script exits non‑zero
if ES doesn't become healthy; implement a loop that increments attempts (or use
timeout) and after N failures logs an error and exits before calling
catalina.sh, ensuring the container can restart or fail fast instead of hanging
indefinitely.
In `@docker-compose/multiwar/nonwars-to-tomcat-config/common-application.yml`:
- Around line 29-30: Update the config-server base-uri entry in
common-application.yml to include a trailing slash so URI concatenation works
correctly; specifically change the config-server: base-uri (the value used by
UriComponentsBuilder.fromUriString) from "http://config-server:80" to
"http://config-server:80/". This will ensure relative paths like "v1/..." are
appended correctly (e.g., config-server.base-uri).
In `@docker-compose/multiwar/README.md`:
- Line 51: Update the section header "## Excluded (Boot WebFlux —
incompatible(??) with WAR)" by removing the uncertainty markers and making the
incompatibility definitive; replace it with a clear statement such as "##
Excluded (Spring Boot WebFlux — not compatible with WAR packaging)" so the
README unambiguously reflects the documented limitation of WebFlux and WAR
packaging.
In `@docker-compose/multiwar/SHARED_CLASSLOADING_ISSUES.txt`:
- Around line 134-143: The ISSUE numbering is inconsistent: the header reads
"ISSUE 10" while the bullet items are labeled "8a/8b/8c"; update the identifiers
to be consistent by either changing the header to "ISSUE 8" or renumbering the
sub-items to "10a/10b/10c" (and update any cross-references or indexes that
mention these IDs), ensuring the header and each sub-item label match (e.g.,
header "ISSUE 10" with sub-items "10a/10b/10c" or vice versa) so readers and
tooling won't be confused.
In `@docker-compose/multiwar/wars.sh`:
- Around line 3-14: The header block at the top of wars.sh still says
"war-deploy.sh" which is stale; update the header comment to reference the
actual script name "wars.sh" (and any internal references like usage lines
showing "./war-deploy.sh") and also adjust the --help handler that prints those
header lines so it emits the corrected script name; search for the header
comment text and the "--help" handler logic in wars.sh and replace occurrences
of "war-deploy.sh" with "wars.sh" (or parameterize the script name from $0) so
the help output matches the real filename.
- Around line 191-199: The code only strips a single trailing whitespace from
the parsed name (variable key), so CRs or multiple trailing spaces can remain;
update the trimming to remove all trailing whitespace/CR characters when
populating gridsuite_services. Replace the current `${key%%[[:space:]]}` with a
full-trim approach (for example, enable extglob and use `${key%%+([[:space:]])}`
or pipe key through `sed 's/[[:space:]]\+$//'`) and apply it in both places
where key is set so gridsuite_services receives clean keys.
---
Nitpick comments:
In `@docker-compose/multiwar/docker-compose.multiwar.yml`:
- Around line 53-152: Extract the repeated common service configuration
(JAVA_TOOL_OPTIONS, command, sysctls, memswap_limit, deploy.resources.limits)
into a YAML anchor (e.g. &svc-defaults) placed near the top of the services
block, then reference it in each service (config-server,
config-notification-server, study-notification-server,
directory-notification-server, gateway) using the merge key (<<: *svc-defaults);
ensure the anchor contains the exact keys to be shared and keep service-specific
keys like image, ports, volumes and any extra mounts outside the merged block so
each service still overrides or extends as needed.
- Around line 6-10: The tomcat service currently uses a floating tag "image:
tomcat:10.1-jdk21" which must be pinned by digest for reproducibility; locate
the tomcat service block (the "tomcat" service and the "image:
tomcat:10.1-jdk21" line) and replace the tag with the corresponding
digest-pinned reference (e.g., "tomcat@sha256:<digest>") obtained from the
Docker registry (docker pull or manifest inspect) so the
docker-compose.multiwar.yml uses an immutable image reference.
In `@docker-compose/multiwar/README.md`:
- Line 3: Remove the user-facing AI generation attribution line "Generated with
little supervision by opus4.6." from the README content so the documentation no
longer references the generation method; if you need to retain provenance, add
that note to the commit message or an internal comment in the repo instead.
- Around line 1-59: Add a new "Limitations & Trade-offs" section to
docker-compose/multiwar/README.md that concisely documents architectural
drawbacks of the multi-WAR approach: mention shared classloader risks
(dependency conflicts, refer to SHARED_CLASSLOADING_ISSUES.txt), single point of
failure (one Tomcat/JVM for all services), no independent scaling or per-service
resource limits, increased debugging complexity and weakened security
boundaries, and recommend this strategy only for dev/testing; also note how to
enable/disable services via the MANIFEST array in war-deploy.sh for operational
context.
- Around line 14-24: The fenced ASCII diagram block in the README lacks a
language identifier; update the diagram's code fence to use the text language by
changing the opening fence from ``` to ```text (and keep the closing fence ```),
so the ASCII art (the "Original server (unmodified) ... report-server.war"
diagram) is rendered correctly and satisfies MD040.
In `@docker-compose/multiwar/wars.sh`:
- Around line 496-509: The loop reading dependencies uses an unused variable `t`
and should be renamed to `_` in the while-read that constructs `shared_deps_xml`
(look for the while IFS=: read -r g a t classifier v; do ... done <
"$deps_dir/_shared.txt") to silence shellcheck; remove or stop assigning the
unused `SHARED_DEPS_FILE` variable (search for its assignment around the 530
area) since it’s never used; and quote the parameter expansion
`${propfile#$res_dir/}` (used later where propfile is stripped of $res_dir) to
prevent accidental globbing by changing to a quoted form so the pattern is
treated literally.
- Around line 451-453: The use of grep -oP is not portable; replace the grep+sed
step with a POSIX-compatible awk that extracts the first non-space token from
each line of "$deps_file" before piping to the existing awk/ sort sequence. For
example, change the pipeline that starts with grep -oP '^\s+\S+' "$deps_file" to
use awk '{ if (match($0,/^[[:space:]]*[^[:space:]]+/)) print
substr($0,RSTART,RLENGTH) }' "$deps_file" | awk -F: 'NF==5 {print
$1":"$2":"$3"::"$4} NF>=6 {print $1":"$2":"$3":"$4":"$5}' | sort -u >
"$deps_dir/${ctx}.parsed", keeping the same variables deps_file, deps_dir and
ctx so the rest of wars.sh remains unchanged.
- Around line 285-370: The function add_shared_lib_autoconfig_exclusions relies
on DEPS_DIR but DEPS_DIR is only set inside find_common_deps, causing
unbound-variable failures when functions are invoked out of order; to fix,
declare and initialize DEPS_DIR alongside the other top-level path globals at
the top of the script (so DEPS_DIR exists even before find_common_deps runs) and
then remove the later assignment to DEPS_DIR from find_common_deps; update
references to DEPS_DIR in add_shared_lib_autoconfig_exclusions and
find_common_deps as needed to ensure the single authoritative initialization is
at the module scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: edc9c00b-ef27-4e3b-bd1a-452aec713251
📒 Files selected for processing (9)
docker-compose/multiwar/.envdocker-compose/multiwar/README.mddocker-compose/multiwar/SHARED_CLASSLOADING_ISSUES.txtdocker-compose/multiwar/docker-compose.multiwar.ymldocker-compose/multiwar/docker-compose.ymldocker-compose/multiwar/itools-config.ymldocker-compose/multiwar/nonwars-to-tomcat-config/common-application.ymldocker-compose/multiwar/server.xmldocker-compose/multiwar/wars.sh
| # Custom server.xml: sets Host startStopThreads="10" for parallel WAR deployment | ||
| - ./server.xml:/usr/local/tomcat/conf/server.xml:Z |
There was a problem hiding this comment.
Comment contradicts the mounted server.xml.
The comment claims server.xml sets Host startStopThreads="10" for parallel WAR deployment, but docker-compose/multiwar/server.xml line 37 uses startStopThreads="1" (sequential), and the script header in wars.sh plus SHARED_CLASSLOADING_ISSUES.txt (ISSUE 4) explicitly explain why sequential deployment is required to avoid the Logback ConcurrentModificationException.
🔧 Proposed fix
- # Custom server.xml: sets Host startStopThreads="10" for parallel WAR deployment
+ # Custom server.xml: sets Host startStopThreads="1" for sequential WAR deployment
+ # (parallel deployment races on the shared Logback LoggerContext — see
+ # SHARED_CLASSLOADING_ISSUES.txt, Issue 4).
- ./server.xml:/usr/local/tomcat/conf/server.xml:ZAlso applies to: 35-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose/multiwar/docker-compose.multiwar.yml` around lines 12 - 13,
The comment incorrectly states that the mounted server.xml configures Host
startStopThreads="10" for parallel WAR deployment; update the comment to reflect
the actual configuration and reasoning by noting that
docker-compose/multiwar/server.xml sets startStopThreads="1" (sequential) to
avoid Logback ConcurrentModificationException described in
SHARED_CLASSLOADING_ISSUES.txt and implemented in wars.sh; change the comment
above the volume mount to accurately describe that server.xml enforces
sequential deployment via startStopThreads="1" and reference the wars.sh
behavior and ISSUE 4 rationale so future readers understand the intentional
sequential setting.
| # Wait for elasticsearch before starting Tomcat: WARs that depend on elasticsearch | ||
| # (study-server, directory-server, case-server, network-conversion-server) fail | ||
| # immediately if ES is not reachable at startup (no built-in retry). | ||
| entrypoint: ["/bin/bash", "-c", "cp /shared-lib/*.jar /usr/local/tomcat/lib/ 2>/dev/null; until curl -sf http://elasticsearch:9200/_cluster/health; do echo 'Waiting for elasticsearch...'; sleep 3; done && catalina.sh run"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm curl is available in the base image.
docker run --rm tomcat:10.1-jdk21 sh -c 'command -v curl && curl --version | head -1' 2>&1 || trueRepository: gridsuite/deployment
Length of output: 109
🌐 Web query:
tomcat 10.1-jdk21 curl available installed
💡 Result:
Yes—Tomcat 10.1 can be run with Java 21 (JDK21). Apache Tomcat 10.1 is designed to run on Java 11 and later [1], so Java 21 is within the supported range [2]. (A related point from release notes: Tomcat 10.1 includes support for virtual threads, which are Java 21+ only.) [3] Tomcat 10.1 is available for download as a binary distribution from the official downloads page [4] (with archives available for older releases) [4]. For example, the Apache distribution site provides version directories like v10.1.54 and includes download files and packaging details [5]. If by “curl available installed” you mean “download it with curl and install”: the official page emphasizes verifying release integrity using the provided OpenPGP signatures and/or SHA-512 checksums after downloading [4]. Example curl download (replace VERSION and MIRROR as appropriate): 1) Pick your target release directory (e.g., v10.1.54) [5]. 2) Download the binary archive from that directory using curl. Note: The exact URL for the tar/zip depends on the chosen Tomcat version and packaging type (e.g., apache-tomcat-VERSION.tar.gz) [5]. For an “installed” option, you can also use the official Tomcat Docker image tag naming scheme that includes JDK21—for example the docker-library project defines a Tomcat 10.1 JDK21-based image using FROM eclipse-temurin:21-jdk-jammy with TOMCAT_VERSION set (example shown for 10.1.54) [6]. The existence of such tags indicates straightforward availability of a JDK21 runtime environment for Tomcat 10.1 [6].
Citations:
- 1: https://tomcat.apache.org/tomcat-10.1-doc/RELEASE-NOTES.txt
- 2: https://tomcat.apache.org/whichversion.html
- 3: https://www.mail-archive.com/announce@apache.org/msg08259.html
- 4: https://tomcat.apache.org/download-10.cgi
- 5: https://downloads.apache.org/tomcat/tomcat-10/v10.1.54/
- 6: https://github.com/docker-library/tomcat/blob/master/10.1/jdk21/temurin-jammy/Dockerfile
🏁 Script executed:
# Check if there's an elasticsearch service definition in the repo with healthcheck
find . -name "*.yml" -o -name "*.yaml" | head -20Repository: gridsuite/deployment
Length of output: 1016
🏁 Script executed:
# Look for elasticsearch service definition and healthchecks
rg -t yaml "elasticsearch" --max-count=20Repository: gridsuite/deployment
Length of output: 3687
🏁 Script executed:
# Get the elasticsearch service definition with context
sed -n '/^ elasticsearch:/,/^ [^ ]/p' docker-compose/technical/docker-compose.technical.yml | head -40Repository: gridsuite/deployment
Length of output: 572
🏁 Script executed:
# Check multiwar compose file for service dependencies
cat -n docker-compose/multiwar/docker-compose.multiwar.yml | head -60Repository: gridsuite/deployment
Length of output: 3950
🌐 Web query:
eclipse-temurin:21-jdk-jammy curl available
💡 Result:
Yes—eclipse-temurin:21-jdk-jammy includes curl. The official Adoptium/Eclipse Temurin container Dockerfile for 21/jdk on Ubuntu jammy explicitly installs curl via apt-get (and documents it as “curl required for historical reasons”).[1] The Docker Hub “eclipse-temurin” image documentation also notes that when you run the container and add certificates, they are available for “curl and other CLI tools,” implying curl is present in the image.[2] If you still want to verify in your environment, run inside the container: docker run --rm -it eclipse-temurin:21-jdk-jammy sh -lc 'command -v curl && curl --version'
Citations:
- 1: https://github.com/adoptium/containers/blob/a475d69551c1fa735bb4bd1570b7c908691525b2/21/jdk/ubuntu/jammy/Dockerfile
- 2: https://hub.docker.com/_/eclipse-temurin
Elasticsearch wait loop has no timeout and will hang indefinitely.
until curl -sf http://elasticsearch:9200/_cluster/health; do sleep 3; done retries forever. If ES never comes up (bad config, image pull issue, OOM), the Tomcat container hangs in the entrypoint indefinitely and restart: unless-stopped won't help — the process never exits. Bound retries to prevent hanging:
Suggested fix
- entrypoint: ["/bin/bash", "-c", "cp /shared-lib/*.jar /usr/local/tomcat/lib/ 2>/dev/null; until curl -sf http://elasticsearch:9200/_cluster/health; do echo 'Waiting for elasticsearch...'; sleep 3; done && catalina.sh run"]
+ entrypoint:
+ - /bin/bash
+ - -c
+ - |
+ cp /shared-lib/*.jar /usr/local/tomcat/lib/ 2>/dev/null
+ for i in $(seq 1 60); do
+ curl -sf http://elasticsearch:9200/_cluster/health && break
+ echo "Waiting for elasticsearch... ($i/60)"
+ sleep 3
+ done
+ curl -sf http://elasticsearch:9200/_cluster/health || { echo "Elasticsearch never became ready"; exit 1; }
+ exec catalina.sh run📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Wait for elasticsearch before starting Tomcat: WARs that depend on elasticsearch | |
| # (study-server, directory-server, case-server, network-conversion-server) fail | |
| # immediately if ES is not reachable at startup (no built-in retry). | |
| entrypoint: ["/bin/bash", "-c", "cp /shared-lib/*.jar /usr/local/tomcat/lib/ 2>/dev/null; until curl -sf http://elasticsearch:9200/_cluster/health; do echo 'Waiting for elasticsearch...'; sleep 3; done && catalina.sh run"] | |
| # Wait for elasticsearch before starting Tomcat: WARs that depend on elasticsearch | |
| # (study-server, directory-server, case-server, network-conversion-server) fail | |
| # immediately if ES is not reachable at startup (no built-in retry). | |
| entrypoint: | |
| - /bin/bash | |
| - -c | |
| - | | |
| cp /shared-lib/*.jar /usr/local/tomcat/lib/ 2>/dev/null | |
| for i in $(seq 1 60); do | |
| curl -sf http://elasticsearch:9200/_cluster/health && break | |
| echo "Waiting for elasticsearch... ($i/60)" | |
| sleep 3 | |
| done | |
| curl -sf http://elasticsearch:9200/_cluster/health || { echo "Elasticsearch never became ready"; exit 1; } | |
| exec catalina.sh run |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose/multiwar/docker-compose.multiwar.yml` around lines 38 - 41,
The entrypoint's Elasticsearch wait loop (the entrypoint array that runs curl
and then catalina.sh run) can hang forever; change it to bound the wait with a
max retry count or a timeout so the script exits non‑zero if ES doesn't become
healthy; implement a loop that increments attempts (or use timeout) and after N
failures logs an error and exits before calling catalina.sh, ensuring the
container can restart or fail fast instead of hanging indefinitely.
| config-server: | ||
| base-uri: http://config-server:80 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find how gridsuite.services.config-server.base-uri is consumed across the
# backend servers. If the same UriComponentsBuilder.fromUriString pattern is
# used as for the other services, the trailing slash matters.
rg -nP -C3 'config-server' --type=java | rg -nP -C3 'base[-_]?uri|baseUri|@Value\s*\(' | head -200
rg -nP -C3 'fromUriString' --type=java | head -100Repository: gridsuite/deployment
Length of output: 362
🏁 Script executed:
#!/bin/bash
# First, explore the repository structure to understand layout
echo "=== Repository structure ==="
find . -maxdepth 2 -type f -name "*.sh" | head -20
find . -maxdepth 3 -type d -name "src" -o -type d -name "java"
ls -la | head -30Repository: gridsuite/deployment
Length of output: 742
🏁 Script executed:
#!/bin/bash
# Find the wars.sh file mentioned in the review
find . -name "wars.sh" -type fRepository: gridsuite/deployment
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Search for config-server references broadly without type filter
rg -i 'config-server' --type-list | head -5
rg 'config-server' -C3 | head -100Repository: gridsuite/deployment
Length of output: 7823
🏁 Script executed:
#!/bin/bash
# Search for how base-uri properties are used across the codebase
rg 'base-uri|base_uri|baseUri' -C2 | head -150Repository: gridsuite/deployment
Length of output: 12368
🏁 Script executed:
#!/bin/bash
# Search for how gridsuite.services.config-server is consumed in the codebase
# Look for `@Value`, `@ConfigurationProperties`, or any config-server property usage
rg 'gridsuite\.services\.config-server|config.?server.*base' -i --type=yml --type=yaml | head -50Repository: gridsuite/deployment
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Look for how services.config-server or base-uri values are injected/used
# Search in property files and Java code
rg 'services\.config-server|\.base-uri|baseUri' -i | head -100Repository: gridsuite/deployment
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Check wars.sh more carefully to understand context of resolve_base_uri
cat docker-compose/multiwar/wars.sh | head -200Repository: gridsuite/deployment
Length of output: 10010
🏁 Script executed:
#!/bin/bash
# Look for config-server references in application-local files
find . -name "application-local.*" -type f | xargs grep -l "config-server" 2>/dev/null | head -20Repository: gridsuite/deployment
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the full common-application.yml file to see context and structure
cat docker-compose/multiwar/nonwars-to-tomcat-config/common-application.ymlRepository: gridsuite/deployment
Length of output: 2141
🏁 Script executed:
#!/bin/bash
# Search for how config-server is actually consumed (look for `@Value` or property injection)
rg 'config-server|gridsuite\.services' | grep -v '.git' | head -100Repository: gridsuite/deployment
Length of output: 5895
Add trailing slash to config-server base-uri.
The config-server entry in common-application.yml is missing a trailing slash: http://config-server:80 instead of http://config-server:80/. All other services in this file (and wars.sh documentation at lines 119–123) confirm that clients use UriComponentsBuilder.fromUriString(baseUri) which concatenates relative paths directly after the base URI. Without the trailing slash, http://config-server:80 + v1/... becomes http://config-server:80v1/... (broken).
🔧 Proposed fix
config-server:
- base-uri: http://config-server:80
+ base-uri: http://config-server:80/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| config-server: | |
| base-uri: http://config-server:80 | |
| config-server: | |
| base-uri: http://config-server:80/ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose/multiwar/nonwars-to-tomcat-config/common-application.yml`
around lines 29 - 30, Update the config-server base-uri entry in
common-application.yml to include a trailing slash so URI concatenation works
correctly; specifically change the config-server: base-uri (the value used by
UriComponentsBuilder.fromUriString) from "http://config-server:80" to
"http://config-server:80/". This will ensure relative paths like "v1/..." are
appended correctly (e.g., config-server.base-uri).
| - Maven 3.8+ | ||
| - Server submodules checked out and buildable in the layout of Gridsuite Aggregator | ||
|
|
||
| ## Excluded (Boot WebFlux — incompatible(??) with WAR) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check WebFlux dependencies in excluded servers to verify incompatibility claims
echo "Checking for WebFlux dependencies in excluded servers..."
for server in config-server config-notification-server directory-notification-server \
merge-notification-server study-notification-server gateway; do
if [ -d "../../../${server}" ]; then
echo "=== ${server} ==="
fd -e xml -e gradle pom.xml build.gradle --exec rg -l "spring-boot-starter-webflux|spring-webflux"
fi
doneRepository: gridsuite/deployment
Length of output: 121
🏁 Script executed:
# Explore the repository structure
git ls-files | head -30Repository: gridsuite/deployment
Length of output: 1210
🏁 Script executed:
# Check the main directory structure
ls -laRepository: gridsuite/deployment
Length of output: 713
🏁 Script executed:
# Look for build files that would indicate where services are defined
fd -e "pom.xml" -e "build.gradle" -o -e "docker-compose.yml" | head -20Repository: gridsuite/deployment
Length of output: 181
🏁 Script executed:
# Read the multiwar README to understand context
cat docker-compose/multiwar/README.mdRepository: gridsuite/deployment
Length of output: 1830
🏁 Script executed:
# Check if any service source code or configs are in this repo that might reference WebFlux
rg -i "webflux|spring-boot-starter-web" --type markdown --type ymlRepository: gridsuite/deployment
Length of output: 92
🌐 Web query:
Spring Boot WebFlux WAR packaging compatibility servlet container
💡 Result:
Spring Boot WebFlux applications are not compatible with WAR packaging for deployment to servlet containers. Official Spring Boot documentation explicitly states: "Because Spring WebFlux does not strictly depend on the Servlet API and applications are deployed by default on an embedded Reactor Netty server, War deployment is not supported for WebFlux applications."[1][2][3][4]. This position is consistent across versions, including recent 3.3.x documentation[2][4], and Spring Boot team members have confirmed no plans to support it due to technical challenges like requiring Servlet 3.1+ with async I/O and risks of mixing blocking/non-blocking code[5][6]. Spring Boot recommends packaging WebFlux apps as executable JARs with embedded Netty[5][6]. While plain Spring Framework WebFlux supports WAR deployment via AbstractReactiveWebInitializer on Servlet 3.1+ containers[7][8][9], this is not integrated with Spring Boot's auto-configuration and leads to errors or unsupported behavior when attempted[5][10][11][12]. No changes noted as of 2026[13][14].
Citations:
- 1: https://docs.spring.io/spring-boot/docs/2.1.6.RELEASE/reference/html/howto-traditional-deployment.html
- 2: https://docs.spring.vmware.com/spring-boot/3.3-SNAPSHOT/how-to/deployment/traditional-deployment.html
- 3: https://docs.spring.io/spring-boot/how-to/deployment/traditional-deployment.html
- 4: https://docs.spring.vmware.com/spring-boot/3.3/reference/web/reactive.html
- 5: https://stackoverflow.com/questions/68202729/spring-webflux-how-to-create-a-tomcat-war-for-deployment
- 6: Mention in docs that WebFlux is not supported in WAR deployment mode spring-projects/spring-boot#12455
- 7: https://stackoverflow.com/questions/54647492/can-i-deploy-a-spring-webflux-application-as-a-war
- 8: https://docs.spring.io/spring-framework/reference/web/webflux.html
- 9: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/server/adapter/AbstractReactiveWebInitializer.html
- 10: https://stackoverflow.com/questions/46714809/spring-web-reactive-not-working-as-war-file-deployed-on-tomcat-8-5
- 11: https://stackoverflow.com/questions/46736435/running-spring-webflux-application-as-a-war
- 12: https://stackoverflow.com/questions/71422339/how-to-deploy-a-spring-boot-webflux-application-to-tomcat-standalone-server
- 13: https://dev.to/jtorchia/jakarta-ee-vs-spring-boot-in-2026-i-migrated-a-production-backend-and-the-tradeoffs-arent-what-5hin
- 14: Web application could not be started as there was no org.ReactiveWebServerFactory bean defined can be thrown due to missing condition on NettyReactiveWebServerAutoConfiguration spring-projects/spring-boot#49693
🌐 Web query:
Spring Cloud Config Server WebFlux Spring Cloud Gateway architecture
💡 Result:
Spring Cloud Config Server WebFlux + Spring Cloud Gateway: how the architecture typically fits together (and where WebFlux can be tricky). 1) What each component is (and what protocol it speaks) - Spring Cloud Config Server exposes an HTTP resource-based API for external configuration (name/value pairs or YAML) and is embeddable in a Spring Boot app using @EnableConfigServer. It runs as a Spring Boot application (default port 8080; commonly 8888). [1] - Spring Cloud Gateway Server WebFlux is a reactive (Spring WebFlux/Project Reactor) gateway; its configuration (routes, predicates, filters) is driven by RouteDefinitionLocator/RouteDefinition and routes are matched via Spring WebFlux HandlerMapping infrastructure. [2][3][4] 2) Canonical “centralized config + reactive gateway” architecture A typical setup is: - Config Server (Spring Boot app): backs centralized config and serves it over HTTP. - Gateway (Spring Cloud Gateway on WebFlux): routes incoming client requests to downstream services (often discovered via Eureka, though that’s a separate concern). Gateway is itself reactive and expects WebFlux runtime semantics. [2][4][3] - Each downstream microservice (including Gateway itself if desired) is a Config Client: it fetches its own configuration from Config Server at startup using the Config Client support (connects to Config Server via spring.cloud.config.uri or spring.config.import=optional:configserver:...). [5][6] Key idea: Config Server is not “wired into” Gateway request routing at the WebFlux layer; instead, it provides configuration to the apps that need it (Gateway and downstream services). Config Server is an HTTP backend for configuration, not a routing component for Gateway. [1][5] 3) Where “Config Server + WebFlux” can be confusing - If you add Spring WebFlux starter(s) to Config Server, you may run into servlet-vs-reactive controller differences. A Stack Overflow report notes that the Config Server ResourceController can rely on ServletWebRequest, and therefore may not work properly with a reactive stack out of the box. [7] So for production-grade architectures, treat Config Server as a server that provides an HTTP API for config, and ensure its own web stack/controller compatibility matches the Spring Cloud Config version you’re using (don’t assume that adding spring-boot-starter-webflux will automatically make every endpoint reactive). [7][1] 4) How Gateway configuration relates to Config Server - Spring Cloud Gateway’s route configuration is driven by route definitions (RouteDefinitionLocator) and can be provided via properties (default PropertiesRouteDefinitionLocator loads from Spring Boot @ConfigurationProperties). [3] - Therefore, the most common approach is: a) Put Gateway route settings (e.g., spring.cloud.gateway.routes.* or equivalent RouteLocator configuration) in the config repository. b) Have the Gateway application fetch those settings from Config Server as part of its normal Config Client startup. This keeps Gateway reactive (it still runs on WebFlux) while config is supplied via the Config Server’s HTTP API. [3][1][5] 5) A concrete “wiring” pattern (conceptual) - Config Server: serves configuration artifacts for each application name/profile over HTTP. [1] - Gateway app: starts, connects to Config Server, loads properties (including Gateway route/predicate/filter config), then starts routing requests reactively. [5][3][4] If you tell me your exact versions (Spring Boot + Spring Cloud release train + Spring Cloud Config/Gateway versions) and whether you want Gateway routes to come from Config Server Git files or from discovery/database, I can draft an example application.yml/bootstrapping layout for both Config Server and Gateway.
Citations:
- 1: https://docs.spring.io/spring-cloud-config/reference/server.html
- 2: https://docs.spring.io/spring-cloud-gateway/reference/spring-cloud-gateway-server-webflux.html
- 3: https://docs.spring.io/spring-cloud-gateway/reference/spring-cloud-gateway-server-webflux/configuration.html
- 4: https://cloud.spring.io/spring-cloud-gateway/reference/html/
- 5: https://docs.spring.io/spring-cloud-config/reference/client.html
- 6: https://spring.io/guides/gs/centralized-configuration
- 7: https://stackoverflow.com/questions/78075701/broken-resourcecontroller-in-spring-cloud-config-server-with-springboot-reactive
Remove the uncertainty markers from the WebFlux incompatibility statement.
The exclusion is correct and well-documented: Spring Boot officially does not support WAR packaging for WebFlux applications because WebFlux depends on an embedded Reactor Netty server and does not integrate with the Servlet API. Replace the "(??) " with a definitive statement since this is a documented limitation, not an uncertain one.
Consider updating to: "Excluded (Spring Boot WebFlux — not compatible with WAR packaging)"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose/multiwar/README.md` at line 51, Update the section header "##
Excluded (Boot WebFlux — incompatible(??) with WAR)" by removing the uncertainty
markers and making the incompatibility definitive; replace it with a clear
statement such as "## Excluded (Spring Boot WebFlux — not compatible with WAR
packaging)" so the README unambiguously reflects the documented limitation of
WebFlux and WAR packaging.
| ISSUE 10: Maven build failures | ||
| ───────────────────────────── | ||
| 8a) Thread contention: -T2.0C (40 threads on 20 cores) caused Maven lock | ||
| contention. Fixed: -T4. | ||
| 8b) Missing artifact com.bea.xml:jsr173-ri:1.0 (transitive of stax-utils). | ||
| Fixed: wildcard <exclusions> on all shared-deps POM entries since | ||
| _shared.txt is already the full transitive closure. | ||
| 8c) Empty WARs: packagingExcludes=WEB-INF/lib/*.jar removed everything. | ||
| maven-war-plugin doesn't support ! negation in packagingExcludes. | ||
| Fixed: switched to packagingIncludes (whitelist approach). |
There was a problem hiding this comment.
ISSUE numbering mismatch.
The header says "ISSUE 10" but the sub-items below are labeled 8a / 8b / 8c.
🔧 Proposed fix
ISSUE 10: Maven build failures
─────────────────────────────
-8a) Thread contention: -T2.0C (40 threads on 20 cores) caused Maven lock
+10a) Thread contention: -T2.0C (40 threads on 20 cores) caused Maven lock
contention. Fixed: -T4.
-8b) Missing artifact com.bea.xml:jsr173-ri:1.0 (transitive of stax-utils).
+10b) Missing artifact com.bea.xml:jsr173-ri:1.0 (transitive of stax-utils).
Fixed: wildcard <exclusions> on all shared-deps POM entries since
_shared.txt is already the full transitive closure.
-8c) Empty WARs: packagingExcludes=WEB-INF/lib/*.jar removed everything.
+10c) Empty WARs: packagingExcludes=WEB-INF/lib/*.jar removed everything.
maven-war-plugin doesn't support ! negation in packagingExcludes.
Fixed: switched to packagingIncludes (whitelist approach).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose/multiwar/SHARED_CLASSLOADING_ISSUES.txt` around lines 134 -
143, The ISSUE numbering is inconsistent: the header reads "ISSUE 10" while the
bullet items are labeled "8a/8b/8c"; update the identifiers to be consistent by
either changing the header to "ISSUE 8" or renumbering the sub-items to
"10a/10b/10c" (and update any cross-references or indexes that mention these
IDs), ensuring the header and each sub-item label match (e.g., header "ISSUE 10"
with sub-items "10a/10b/10c" or vice versa) so readers and tooling won't be
confused.
| # war-deploy.sh — Build GridSuite microservices as WAR files for docker-compose. | ||
| # Zero modifications to original server source files. | ||
| # | ||
| # Word of Caution, the whole script with comments (except this sentence) | ||
| # has been generated by opus4.6 without much look at the details. | ||
| # | ||
| # Usage: | ||
| # ./war-deploy.sh [OPTIONS] | ||
| # | ||
| # Options: | ||
| # --only SRV Only process this server (repeatable) | ||
| # --help Show this help |
There was a problem hiding this comment.
Stale filename in header comment.
The header refers to war-deploy.sh, but the file is named wars.sh. The --help handler (line 56) also extracts these lines verbatim, so users will see the wrong command name.
🔧 Proposed fix
-# war-deploy.sh — Build GridSuite microservices as WAR files for docker-compose.
+# wars.sh — Build GridSuite microservices as WAR files for docker-compose.
# Zero modifications to original server source files.
...
# Usage:
-# ./war-deploy.sh [OPTIONS]
+# ./wars.sh [OPTIONS]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # war-deploy.sh — Build GridSuite microservices as WAR files for docker-compose. | |
| # Zero modifications to original server source files. | |
| # | |
| # Word of Caution, the whole script with comments (except this sentence) | |
| # has been generated by opus4.6 without much look at the details. | |
| # | |
| # Usage: | |
| # ./war-deploy.sh [OPTIONS] | |
| # | |
| # Options: | |
| # --only SRV Only process this server (repeatable) | |
| # --help Show this help | |
| # wars.sh — Build GridSuite microservices as WAR files for docker-compose. | |
| # Zero modifications to original server source files. | |
| # | |
| # Word of Caution, the whole script with comments (except this sentence) | |
| # has been generated by opus4.6 without much look at the details. | |
| # | |
| # Usage: | |
| # ./wars.sh [OPTIONS] | |
| # | |
| # Options: | |
| # --only SRV Only process this server (repeatable) | |
| # --help Show this help |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose/multiwar/wars.sh` around lines 3 - 14, The header block at the
top of wars.sh still says "war-deploy.sh" which is stale; update the header
comment to reference the actual script name "wars.sh" (and any internal
references like usage lines showing "./war-deploy.sh") and also adjust the
--help handler that prints those header lines so it emits the corrected script
name; search for the header comment text and the "--help" handler logic in
wars.sh and replace occurrences of "war-deploy.sh" with "wars.sh" (or
parameterize the script name from $0) so the help output matches the real
filename.
| if [[ "$line" =~ ^[[:space:]]+-[[:space:]]*name:[[:space:]]*(.+) ]]; then | ||
| local key="${BASH_REMATCH[1]}" | ||
| key="${key%%[[:space:]]}" | ||
| [[ "$current_section" == "gridsuite" ]] && gridsuite_services+=("$key") | ||
| elif [[ "$line" =~ ^[[:space:]]+name:[[:space:]]*(.+) && "$current_section" == "gridsuite" ]]; then | ||
| local key="${BASH_REMATCH[1]}" | ||
| key="${key%%[[:space:]]}" | ||
| gridsuite_services+=("$key") | ||
| fi |
There was a problem hiding this comment.
Trailing-whitespace trim only strips a single character.
${key%%[[:space:]]} uses a glob pattern (not a regex), and [[:space:]] is a single-character class — so even with %% this removes at most one trailing whitespace. Trailing CR (e.g., CRLF files) or multiple spaces will leak into the YAML key list and later URI generation.
🔧 Proposed fix
- if [[ "$line" =~ ^[[:space:]]+-[[:space:]]*name:[[:space:]]*(.+) ]]; then
- local key="${BASH_REMATCH[1]}"
- key="${key%%[[:space:]]}"
+ if [[ "$line" =~ ^[[:space:]]+-[[:space:]]*name:[[:space:]]*(.+) ]]; then
+ local key="${BASH_REMATCH[1]}"
+ key="${key%"${key##*[![:space:]]}"}"
[[ "$current_section" == "gridsuite" ]] && gridsuite_services+=("$key")
elif [[ "$line" =~ ^[[:space:]]+name:[[:space:]]*(.+) && "$current_section" == "gridsuite" ]]; then
local key="${BASH_REMATCH[1]}"
- key="${key%%[[:space:]]}"
+ key="${key%"${key##*[![:space:]]}"}"
gridsuite_services+=("$key")
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ "$line" =~ ^[[:space:]]+-[[:space:]]*name:[[:space:]]*(.+) ]]; then | |
| local key="${BASH_REMATCH[1]}" | |
| key="${key%%[[:space:]]}" | |
| [[ "$current_section" == "gridsuite" ]] && gridsuite_services+=("$key") | |
| elif [[ "$line" =~ ^[[:space:]]+name:[[:space:]]*(.+) && "$current_section" == "gridsuite" ]]; then | |
| local key="${BASH_REMATCH[1]}" | |
| key="${key%%[[:space:]]}" | |
| gridsuite_services+=("$key") | |
| fi | |
| if [[ "$line" =~ ^[[:space:]]+-[[:space:]]*name:[[:space:]]*(.+) ]]; then | |
| local key="${BASH_REMATCH[1]}" | |
| key="${key%"${key##*[![:space:]]}"}" | |
| [[ "$current_section" == "gridsuite" ]] && gridsuite_services+=("$key") | |
| elif [[ "$line" =~ ^[[:space:]]+name:[[:space:]]*(.+) && "$current_section" == "gridsuite" ]]; then | |
| local key="${BASH_REMATCH[1]}" | |
| key="${key%"${key##*[![:space:]]}"}" | |
| gridsuite_services+=("$key") | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose/multiwar/wars.sh` around lines 191 - 199, The code only strips
a single trailing whitespace from the parsed name (variable key), so CRs or
multiple trailing spaces can remain; update the trimming to remove all trailing
whitespace/CR characters when populating gridsuite_services. Replace the current
`${key%%[[:space:]]}` with a full-trim approach (for example, enable extglob and
use `${key%%+([[:space:]])}` or pipe key through `sed 's/[[:space:]]\+$//'`) and
apply it in both places where key is set so gridsuite_services receives clean
keys.
1 participant
PR Summary
horrible POC of 2.5g ram running loadflows and security-analysis on country size neworks