mirror of
https://github.com/aljazceru/goose.git
synced 2026-02-07 23:54:26 +01:00
Cleanup Temporal debug files (#3089)
This commit is contained in:
@@ -1,81 +0,0 @@
|
||||
# TemporalScheduler gRPC Detection Fix - COMPLETED ✅
|
||||
|
||||
## Critical Issue Resolved
|
||||
**Error**: `Port 7233 is already in use by something other than a Temporal server.`
|
||||
|
||||
**Root Cause**: The `check_temporal_server()` method was trying to communicate with Temporal server using HTTP protocol on port 7233, but Temporal server actually uses **gRPC protocol** on that port.
|
||||
|
||||
## The Problem
|
||||
```rust
|
||||
// OLD (BROKEN) - Trying HTTP on gRPC port
|
||||
async fn check_temporal_server(&self) -> bool {
|
||||
match self.http_client.get(format!("{}/api/v1/namespaces", TEMPORAL_SERVER_URL)).send().await {
|
||||
Ok(response) => response.status().is_success(),
|
||||
Err(_) => false,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This would always return `false` even when a perfectly good Temporal server was running, causing the scheduler to think port 7233 was occupied by "something other than a Temporal server."
|
||||
|
||||
## The Solution
|
||||
```rust
|
||||
// NEW (WORKING) - Multi-protocol detection
|
||||
async fn check_temporal_server(&self) -> bool {
|
||||
// First try the web UI (which uses HTTP)
|
||||
if let Ok(response) = self.http_client.get("http://localhost:8233/").send().await {
|
||||
if response.status().is_success() {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// Alternative: check if we can establish a TCP connection to the gRPC port
|
||||
use std::net::SocketAddr;
|
||||
use std::time::Duration;
|
||||
|
||||
let addr: SocketAddr = "127.0.0.1:7233".parse().unwrap();
|
||||
match std::net::TcpStream::connect_timeout(&addr, Duration::from_secs(2)) {
|
||||
Ok(_) => {
|
||||
info!("Detected Temporal server on port 7233 (gRPC connection successful)");
|
||||
true
|
||||
}
|
||||
Err(_) => false,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## How It Works Now
|
||||
1. **HTTP Check**: First tries to connect to Temporal Web UI on port 8233 (HTTP)
|
||||
2. **gRPC Check**: If that fails, tries TCP connection to gRPC port 7233
|
||||
3. **Smart Detection**: If either succeeds, recognizes it as a valid Temporal server
|
||||
4. **Connection**: Connects to existing server instead of failing with port conflict
|
||||
|
||||
## Test Results
|
||||
```
|
||||
✅ Temporal server detection test completed
|
||||
Temporal server detected: true
|
||||
🎉 SUCCESS: Found existing Temporal server!
|
||||
The scheduler will connect to it instead of failing
|
||||
```
|
||||
|
||||
## Verification
|
||||
- ✅ All unit tests pass
|
||||
- ✅ Code compiles without warnings
|
||||
- ✅ Clippy checks pass
|
||||
- ✅ Real-world detection confirmed with existing server
|
||||
- ✅ Port conflict logic verified
|
||||
|
||||
## Impact
|
||||
- **No more false negatives**: Properly detects existing Temporal servers
|
||||
- **No more crashes**: Connects to existing infrastructure gracefully
|
||||
- **Better reliability**: Works with real Temporal deployments
|
||||
- **Production ready**: Handles gRPC protocol correctly
|
||||
|
||||
## Files Modified
|
||||
- `crates/goose/src/temporal_scheduler.rs` - Fixed detection logic
|
||||
- Added comprehensive test for gRPC detection
|
||||
|
||||
## Commits
|
||||
- **316bc12189**: Fix: Properly detect existing Temporal server using correct protocol
|
||||
|
||||
The TemporalScheduler now correctly handles the protocol differences and will successfully connect to existing Temporal servers instead of failing with misleading port conflict errors! 🎉
|
||||
@@ -1,125 +0,0 @@
|
||||
# TemporalScheduler Port Conflict Fix - COMPLETED ✅
|
||||
|
||||
## Issue Fixed
|
||||
The TemporalScheduler was crashing when Temporal services were already running, with errors like:
|
||||
```
|
||||
Error: Scheduler internal error: Port 7233 is already in use. Another Temporal server may be running.
|
||||
```
|
||||
|
||||
This caused the goosed server to fail to start, preventing the desktop application from working.
|
||||
|
||||
## Root Cause
|
||||
The original logic would:
|
||||
1. Check if ports 7233 and 8080 were in use
|
||||
2. If in use, immediately return an error
|
||||
3. Never attempt to connect to existing services
|
||||
|
||||
This was problematic because:
|
||||
- Users might have Temporal services already running
|
||||
- Multiple instances of the application couldn't coexist
|
||||
- The scheduler couldn't leverage existing infrastructure
|
||||
|
||||
## Solution Implemented
|
||||
|
||||
### 1. **Enhanced Service Detection Logic**
|
||||
- **File**: `crates/goose/src/temporal_scheduler.rs`
|
||||
- **Method**: `ensure_services_running()`
|
||||
- **Improvement**: Now checks both services comprehensively before deciding what to start
|
||||
|
||||
```rust
|
||||
async fn ensure_services_running(&self) -> Result<(), SchedulerError> {
|
||||
// First, check if both services are already running
|
||||
let temporal_running = self.check_temporal_server().await;
|
||||
let go_service_running = self.health_check().await.unwrap_or(false);
|
||||
|
||||
if temporal_running && go_service_running {
|
||||
info!("Both Temporal server and Go service are already running");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Handle various combinations of service states...
|
||||
}
|
||||
```
|
||||
|
||||
### 2. **Smart Port Conflict Resolution**
|
||||
- **Temporal Server**: If port 7233 is in use, check if it's actually a Temporal server we can connect to
|
||||
- **Go Service**: If port 8080 is in use, check if it's our Go service we can connect to
|
||||
- **Only error if ports are used by incompatible services**
|
||||
|
||||
```rust
|
||||
async fn start_temporal_server(&self) -> Result<(), SchedulerError> {
|
||||
if self.check_port_in_use(7233).await {
|
||||
// Port is in use - check if it's a Temporal server we can connect to
|
||||
if self.check_temporal_server().await {
|
||||
info!("Port 7233 is in use by a Temporal server we can connect to");
|
||||
return Ok(());
|
||||
} else {
|
||||
return Err(SchedulerError::SchedulerInternalError(
|
||||
"Port 7233 is already in use by something other than a Temporal server.".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
// ... start new server if needed
|
||||
}
|
||||
```
|
||||
|
||||
### 3. **Comprehensive Testing**
|
||||
Added 4 unit tests:
|
||||
- `test_sessions_method_exists_and_compiles` - Verifies sessions() method works
|
||||
- `test_sessions_method_signature` - Compile-time signature verification
|
||||
- `test_port_check_functionality` - Tests port checking logic
|
||||
- `test_service_status_checking` - Tests service detection methods
|
||||
|
||||
### 4. **Improved Error Messages**
|
||||
- Clear distinction between "port in use by compatible service" vs "port in use by incompatible service"
|
||||
- Better logging for debugging service startup issues
|
||||
- Informative messages about what services are detected
|
||||
|
||||
## Key Behavioral Changes
|
||||
|
||||
### Before (❌ Problematic)
|
||||
```
|
||||
1. Check if port 7233 is in use
|
||||
2. If yes → Error: "Port already in use"
|
||||
3. Application crashes
|
||||
```
|
||||
|
||||
### After (✅ Fixed)
|
||||
```
|
||||
1. Check if port 7233 is in use
|
||||
2. If yes → Check if it's a Temporal server
|
||||
3. If it's a Temporal server → Connect to it
|
||||
4. If it's not a Temporal server → Error with specific message
|
||||
5. If port is free → Start new Temporal server
|
||||
```
|
||||
|
||||
## Files Modified
|
||||
- `crates/goose/src/temporal_scheduler.rs` - Main implementation
|
||||
- Added comprehensive test suite
|
||||
- Created verification script: `test_port_conflict_fix.sh`
|
||||
|
||||
## Verification Results
|
||||
✅ All unit tests pass
|
||||
✅ Code compiles without warnings
|
||||
✅ Clippy checks pass
|
||||
✅ Service detection logic verified
|
||||
✅ Port checking functionality confirmed
|
||||
|
||||
## Commits Made
|
||||
1. **cccbba4fd9**: Fix: Improve TemporalScheduler service detection and port conflict handling
|
||||
2. **c645a4941f**: Fix: Connect to existing Temporal services instead of erroring on port conflicts
|
||||
|
||||
## Impact
|
||||
- **No more crashes** when Temporal services are already running
|
||||
- **Better resource utilization** by connecting to existing services
|
||||
- **Improved user experience** - application starts reliably
|
||||
- **Enhanced debugging** with better error messages and logging
|
||||
- **Production ready** - handles real-world deployment scenarios
|
||||
|
||||
## Testing
|
||||
Run the verification script to confirm all fixes are working:
|
||||
```bash
|
||||
./test_port_conflict_fix.sh
|
||||
```
|
||||
|
||||
The TemporalScheduler now gracefully handles existing services and provides a robust, production-ready scheduling solution.
|
||||
@@ -1,114 +0,0 @@
|
||||
#!/bin/bash
|
||||
|
||||
# Test script for Temporal Scheduler Integration
|
||||
# This script tests the complete Phase 2 implementation
|
||||
|
||||
set -e
|
||||
|
||||
echo "🚀 Testing Temporal Scheduler Integration - Phase 2"
|
||||
echo "=================================================="
|
||||
|
||||
# Colors for output
|
||||
RED='\033[0;31m'
|
||||
GREEN='\033[0;32m'
|
||||
YELLOW='\033[1;33m'
|
||||
NC='\033[0m' # No Color
|
||||
|
||||
# Function to print status
|
||||
print_status() {
|
||||
echo -e "${GREEN}✅ $1${NC}"
|
||||
}
|
||||
|
||||
print_warning() {
|
||||
echo -e "${YELLOW}⚠️ $1${NC}"
|
||||
}
|
||||
|
||||
print_error() {
|
||||
echo -e "${RED}❌ $1${NC}"
|
||||
}
|
||||
|
||||
# Check prerequisites
|
||||
echo "🔍 Checking prerequisites..."
|
||||
|
||||
# Check if Temporal CLI is installed
|
||||
if ! command -v temporal &> /dev/null; then
|
||||
print_error "Temporal CLI not found. Install with: brew install temporal"
|
||||
exit 1
|
||||
fi
|
||||
print_status "Temporal CLI found"
|
||||
|
||||
# Check if Go service is built
|
||||
if [ ! -f "temporal-service/temporal-service" ]; then
|
||||
print_error "Temporal service not built. Run: cd temporal-service && ./build.sh"
|
||||
exit 1
|
||||
fi
|
||||
print_status "Temporal service binary found"
|
||||
|
||||
# Check if Rust executor is built
|
||||
if ! command -v goose-scheduler-executor &> /dev/null; then
|
||||
print_error "goose-scheduler-executor not found in PATH"
|
||||
print_warning "Building and installing executor..."
|
||||
cargo build --release --bin goose-scheduler-executor
|
||||
cp target/release/goose-scheduler-executor /usr/local/bin/
|
||||
fi
|
||||
print_status "goose-scheduler-executor found"
|
||||
|
||||
# Build the goose library
|
||||
echo "🔨 Building goose library..."
|
||||
cargo build --lib -p goose
|
||||
print_status "Goose library built successfully"
|
||||
|
||||
# Test 1: Verify trait compilation
|
||||
echo "🧪 Test 1: Verify trait abstraction compiles..."
|
||||
cargo check --lib -p goose
|
||||
print_status "Trait abstraction compiles correctly"
|
||||
|
||||
# Test 2: Verify executor binary works
|
||||
echo "🧪 Test 2: Test executor binary help..."
|
||||
if goose-scheduler-executor --help > /dev/null 2>&1; then
|
||||
print_status "Executor binary responds to --help"
|
||||
else
|
||||
print_error "Executor binary failed help test"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Test 3: Create a test recipe
|
||||
echo "🧪 Test 3: Creating test recipe..."
|
||||
TEST_RECIPE_DIR="/tmp/goose-temporal-test"
|
||||
mkdir -p "$TEST_RECIPE_DIR"
|
||||
|
||||
cat > "$TEST_RECIPE_DIR/test-recipe.yaml" << 'EOF'
|
||||
version: "1.0.0"
|
||||
title: "Temporal Test Recipe"
|
||||
description: "A simple test recipe for Temporal scheduler"
|
||||
prompt: "Say hello and tell me the current time"
|
||||
EOF
|
||||
|
||||
print_status "Test recipe created at $TEST_RECIPE_DIR/test-recipe.yaml"
|
||||
|
||||
# Test 4: Verify the integration compiles with all features
|
||||
echo "🧪 Test 4: Full compilation test..."
|
||||
cargo build --workspace --exclude goose-server --exclude goose-cli
|
||||
print_status "Full workspace builds successfully"
|
||||
|
||||
echo ""
|
||||
echo "🎉 Phase 2 Integration Tests Complete!"
|
||||
echo "======================================"
|
||||
print_status "All tests passed successfully"
|
||||
echo ""
|
||||
echo "📋 What was tested:"
|
||||
echo " ✅ Prerequisites (Temporal CLI, Go service, Rust executor)"
|
||||
echo " ✅ Goose library compilation"
|
||||
echo " ✅ Trait abstraction"
|
||||
echo " ✅ Executor binary functionality"
|
||||
echo " ✅ Test recipe creation"
|
||||
echo " ✅ Full workspace compilation"
|
||||
echo ""
|
||||
echo "🚀 Ready for Phase 3: Migration & Testing"
|
||||
echo ""
|
||||
echo "To test the Temporal scheduler manually:"
|
||||
echo " 1. Set environment: export GOOSE_SCHEDULER_TYPE=temporal"
|
||||
echo " 2. Start services: cd temporal-service && ./start.sh"
|
||||
echo " 3. Use the scheduler factory in your code"
|
||||
echo ""
|
||||
print_status "Phase 2 implementation is ready!"
|
||||
@@ -1,84 +0,0 @@
|
||||
#!/bin/bash
|
||||
|
||||
# Simple test to verify the port conflict fix
|
||||
echo "🧪 Testing TemporalScheduler port conflict fix"
|
||||
echo "=============================================="
|
||||
|
||||
# Check if we're in the right directory
|
||||
if [ ! -f "crates/goose/src/temporal_scheduler.rs" ]; then
|
||||
echo "❌ Please run this script from the goose project root directory"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "✅ Prerequisites check passed"
|
||||
|
||||
# Build the project
|
||||
echo "🔨 Building project..."
|
||||
cargo build --release > /dev/null 2>&1
|
||||
if [ $? -ne 0 ]; then
|
||||
echo "❌ Build failed"
|
||||
exit 1
|
||||
fi
|
||||
echo "✅ Build successful"
|
||||
|
||||
# Run the unit tests to make sure our logic is correct
|
||||
echo "🧪 Running TemporalScheduler unit tests..."
|
||||
cargo test temporal_scheduler::tests --quiet > /dev/null 2>&1
|
||||
if [ $? -eq 0 ]; then
|
||||
echo "✅ All unit tests passed"
|
||||
else
|
||||
echo "❌ Unit tests failed"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Check the code for the specific improvements
|
||||
echo "🔍 Verifying code improvements..."
|
||||
|
||||
# Check that we have the improved service detection logic
|
||||
if grep -q "Port 7233 is in use by a Temporal server we can connect to" crates/goose/src/temporal_scheduler.rs; then
|
||||
echo "✅ Found improved Temporal server detection logic"
|
||||
else
|
||||
echo "❌ Missing improved Temporal server detection logic"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if grep -q "Port 8080 is in use by a Go service we can connect to" crates/goose/src/temporal_scheduler.rs; then
|
||||
echo "✅ Found improved Go service detection logic"
|
||||
else
|
||||
echo "❌ Missing improved Go service detection logic"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Check that we have the comprehensive service status checking
|
||||
if grep -q "First, check if both services are already running" crates/goose/src/temporal_scheduler.rs; then
|
||||
echo "✅ Found comprehensive service status checking"
|
||||
else
|
||||
echo "❌ Missing comprehensive service status checking"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Check that we have proper port checking
|
||||
if grep -q "check_port_in_use" crates/goose/src/temporal_scheduler.rs; then
|
||||
echo "✅ Found port checking functionality"
|
||||
else
|
||||
echo "❌ Missing port checking functionality"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "🎉 All checks passed!"
|
||||
echo "✅ TemporalScheduler now has improved service detection"
|
||||
echo "✅ Port conflicts are handled gracefully"
|
||||
echo "✅ Existing services are detected and connected to"
|
||||
echo "✅ No more crashes when services are already running"
|
||||
|
||||
echo ""
|
||||
echo "📋 Summary of improvements:"
|
||||
echo " • Enhanced ensure_services_running() logic"
|
||||
echo " • Added port conflict detection with service verification"
|
||||
echo " • Improved error handling for various service states"
|
||||
echo " • Added comprehensive unit tests"
|
||||
echo " • Now connects to existing services instead of failing"
|
||||
|
||||
echo ""
|
||||
echo "🚀 The TemporalScheduler is ready for production use!"
|
||||
Reference in New Issue
Block a user