Refactor scripts to support both 'docker compose' and 'docker-compose…

[craig8]

…' commands

[Copilot]

Pull Request Overview

This PR refactors the Docker Compose-related scripts to support both the newer docker compose command (built into Docker) and the older standalone docker-compose command, improving compatibility across different Docker installation versions.

Key Changes:

• Added automatic detection logic to identify and use the available Docker Compose command variant
• Updated all Docker Compose command invocations to use the dynamically detected command
• Enhanced error handling with clear messages when neither command is found

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File	Description
stop.sh	Added detect_docker_compose() function and updated all docker-compose command calls to use the detected variant
run.sh	Added detect_docker_compose() function, updated all command calls, and improved container ID detection logic with fallback handling
docker-compose.yml	Removed trailing whitespace and added ulimits configuration for blazegraph service
.gitignore	Added *.log pattern to ignore log files

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The pipe operator (|) in the command array format won't work as expected. When using array format with ["/bin/bash", "-c"] entrypoint, the pipe needs to be passed as a single string argument to bash. However, tail -f /dev/null will run indefinitely and block the execution of ./run-gridappsd.sh. The logic appears incorrect - this would never reach the script execution. Consider either:

1. Removing tail -f /dev/null | entirely and just use command: ["./run-gridappsd.sh"]
2. Or if the intent is to keep the container alive, the run-gridappsd.sh script should handle that internally based on the AUTOSTART environment variable.

Suggested change

	command: ["tail -f /dev/null | ./run-gridappsd.sh"]
	command: ["./run-gridappsd.sh"]

[Copilot]

The command substitution $(docker ps -a -q) will fail if there are no containers, causing docker rm to run without arguments and print an error message. Consider checking if there are containers first:

container_ids=$(docker ps -a -q)
if [ -n "$container_ids" ]; then
    docker rm $container_ids
else
    echo "No containers to remove."
fi

Suggested change

	docker rm $(docker ps -a -q) 2>/dev/null
	container_ids=$(docker ps -a -q)
	if [ -n "$container_ids" ]; then
	docker rm $container_ids 2>/dev/null
	else
	echo "No containers to remove."
	fi

[Copilot]

Using ls in a command substitution is fragile and subject to word splitting issues. If filenames contain spaces or special characters, this will break. Consider using a more robust approach:

get_compose_files() {
  local files=""
  for f in docker-compose.d/*.yml 2>/dev/null; do
    [ -f "$f" ] && files="$files -f $f"
  done
  echo "-f docker-compose.yml$files"
}

Or use arrays for safer handling of filenames with special characters.

Suggested change

	local files=$( ls -1 docker-compose.d/*yml 2>/dev/null | sed -e 's/^/-f /g' | tr '\n' ' ' )
	local files=""
	for f in docker-compose.d/*.yml 2>/dev/null; do
	[ -f "$f" ] && files="$files -f \"$f\""
	done

[Copilot]

Hardcoded password 'mysecretpassword' in the database URI is a security risk. Even though this is a template file (.dst), it sets a bad precedent. Consider using an environment variable like ${AUTHENTICATOR_PASSWORD:-mysecretpassword} and document that users should change this in production.

Suggested change

	PGRST_DB_URI: postgres://authenticator:mysecretpassword@timescaledb:5432/${TIMESCALE_DB:-gridappsd_ts}
	# WARNING: Change AUTHENTICATOR_PASSWORD in production!
	PGRST_DB_URI: postgres://authenticator:${AUTHENTICATOR_PASSWORD:-mysecretpassword}@timescaledb:5432/${TIMESCALE_DB:-gridappsd_ts}

[Copilot]

Consider using a less specific version pin (e.g., pg16-ts2.24-oss or pg16-latest) or making it configurable via environment variable. The current pin to pg16.11-ts2.24.0-oss may become outdated quickly and could miss important security patches. Alternatively, add a comment explaining why this specific version is required.

Suggested change

	image: timescale/timescaledb-ha:pg16.11-ts2.24.0-oss
	# Use environment variable TIMESCALE_IMAGE to override the TimescaleDB image version.
	# Default is a less specific tag to allow for security updates.
	image: ${TIMESCALE_IMAGE:-timescale/timescaledb-ha:pg16-ts2.24-oss}

[Copilot]

The volume mappings reference paths outside the gridappsd-docker repository (../Centralized/ and ../apps/proven-docker/). This creates a dependency on external directory structure that may not exist for users. Consider either:

1. Including these files in the gridappsd-docker repository
2. Providing clear documentation about required external repositories
3. Adding error handling in the scripts to check if these paths exist before starting

[Copilot]

The build context references ../Centralized/gridappsd_integration which is outside the gridappsd-docker repository. This will fail if users don't have this external repository cloned. See comment on lines 43-46 for the same issue with volume mappings.