Skip to content

Conversation

caspervonb
Copy link
Collaborator

@caspervonb caspervonb commented Oct 7, 2025

Brings install time of nats-server from 2-3 minutes to 2-3 seconds.

@caspervonb caspervonb force-pushed the use-server-binaries-from-nats-dev branch 6 times, most recently from c49a18e to 430e3fd Compare October 7, 2025 10:30
@caspervonb caspervonb requested a review from Copilot October 7, 2025 10:31
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a reusable GitHub Action to optimize NATS server installation in CI workflows, reducing installation time from 2-3 minutes to 2-3 seconds through caching.

  • Replaces manual installation scripts with a centralized GitHub Action
  • Implements caching strategy for NATS server binaries across workflow runs
  • Simplifies workflow configuration by eliminating manual PATH management

Reviewed Changes

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

File Description
.github/actions/setup-nats-server/action.yml New composite action that downloads, caches, and configures NATS server
.github/workflows/test.yml Updated to use the new action and simplified matrix configuration
nats/scripts/install_nats.sh Removed obsolete installation script
nats/tests/utils.py Updated comment to reference official NATS installation docs

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

include:
- nats_version: "main"
continue-on-error: true
nats_version: ["latest"]
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider testing against multiple NATS versions to ensure compatibility. The original configuration tested against specific versions (v2.10.29, v2.11.8) and main branch, which provided better coverage for potential breaking changes.

Suggested change
nats_version: ["latest"]
nats_version: ["v2.10.29", "v2.11.8", "latest", "main"]

Copilot uses AI. Check for mistakes.

run: |
INSTALL_DIR="$HOME/nats-server"
mkdir -p "$INSTALL_DIR"
curl -fsSL https://binaries.nats.dev/nats-io/nats-server/v2@${{ inputs.version }} | PREFIX="$INSTALL_DIR" sh
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Piping curl output directly to shell without verification poses a security risk. Consider downloading the script first, verifying its integrity, then executing it.

Suggested change
curl -fsSL https://binaries.nats.dev/nats-io/nats-server/v2@${{ inputs.version }} | PREFIX="$INSTALL_DIR" sh
INSTALL_SCRIPT="$(mktemp)"
curl -fsSL https://binaries.nats.dev/nats-io/nats-server/v2@${{ inputs.version }} -o "$INSTALL_SCRIPT"
# TODO: (Optional) Verify the integrity of $INSTALL_SCRIPT here (e.g., with sha256sum)
PREFIX="$INSTALL_DIR" sh "$INSTALL_SCRIPT"
rm -f "$INSTALL_SCRIPT"

Copilot uses AI. Check for mistakes.

Comment on lines 28 to 32
# On Windows, create a shim without .exe extension
if [ "$RUNNER_OS" = "Windows" ] && [ -f "$INSTALL_DIR/nats-server.exe" ]; then
echo '#!/bin/bash' > "$INSTALL_DIR/nats-server"
echo 'exec "$(dirname "$0")/nats-server.exe" "$@"' >> "$INSTALL_DIR/nats-server"
chmod +x "$INSTALL_DIR/nats-server"
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Windows shim creation uses bash-specific syntax but may not work correctly on Windows runners. Consider using PowerShell or a more portable approach for Windows compatibility.

Suggested change
# On Windows, create a shim without .exe extension
if [ "$RUNNER_OS" = "Windows" ] && [ -f "$INSTALL_DIR/nats-server.exe" ]; then
echo '#!/bin/bash' > "$INSTALL_DIR/nats-server"
echo 'exec "$(dirname "$0")/nats-server.exe" "$@"' >> "$INSTALL_DIR/nats-server"
chmod +x "$INSTALL_DIR/nats-server"
# On Windows, create a shim without .exe extension using PowerShell
if [ "$RUNNER_OS" = "Windows" ] && [ -f "$INSTALL_DIR/nats-server.exe" ]; then
# Create a CMD shim for Windows
echo '@echo off' > "$INSTALL_DIR/nats-server.cmd"
echo 'set SCRIPT_DIR=%~dp0' >> "$INSTALL_DIR/nats-server.cmd"
echo '"%SCRIPT_DIR%nats-server.exe" %*' >> "$INSTALL_DIR/nats-server.cmd"

Copilot uses AI. Check for mistakes.

@caspervonb caspervonb force-pushed the use-server-binaries-from-nats-dev branch 7 times, most recently from 6f2ee8a to a467549 Compare October 7, 2025 10:49
@caspervonb caspervonb force-pushed the use-server-binaries-from-nats-dev branch from a467549 to 3e13e9a Compare October 7, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant