Review param usage in client_agent

Author: bbockelm

client_agent/apiclient/client_test.go

@@ -29,7 +29,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -72,7 +71,8 @@ func setupTestEnvironment(t *testing.T) (apiClient *apiclient.APIClient, fed *fe
 
 	// Create token for authenticated operations
 	t.Log("setupTestEnvironment: Creating token")
-	viper.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	err := param.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	require.NoError(t, err)
 	issuer, err := config.GetServerIssuerURL()
 	require.NoError(t, err)
 

client_agent/cli_test.go

@@ -34,7 +34,6 @@ import (
 	"time"
 
 	log "github.com/sirupsen/logrus"
-	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -98,7 +97,8 @@ func TestCLIAsyncGet(t *testing.T) {
 	tempDir := t.TempDir()
 
 	// Create token
-	viper.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	err := param.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	require.NoError(t, err)
 	issuer, err := config.GetServerIssuerURL()
 	require.NoError(t, err)
 
@@ -234,7 +234,8 @@ func TestCLIAsyncPut(t *testing.T) {
 
 	// Create token
 	tokenStart := time.Now()
-	viper.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	err := param.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	require.NoError(t, err)
 	issuer, err := config.GetServerIssuerURL()
 	require.NoError(t, err)
 
@@ -348,7 +349,8 @@ func TestCLIJobCommands(t *testing.T) {
 	tempDir := t.TempDir()
 
 	// Create token
-	viper.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	err := param.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	require.NoError(t, err)
 	issuer, err := config.GetServerIssuerURL()
 	require.NoError(t, err)
 

client_agent/integration_test.go

@@ -35,7 +35,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -115,7 +114,8 @@ func TestClientAPIIntegration(t *testing.T) {
 	require.NoError(t, err)
 
 	// Create token for authenticated operations
-	viper.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	err = param.Set(param.IssuerKeysDirectory.GetName(), t.TempDir())
+	require.NoError(t, err)
 	issuer, err := config.GetServerIssuerURL()
 	require.NoError(t, err)
 

client_agent/server.go

@@ -30,13 +30,14 @@ import (
 	"github.com/gin-gonic/gin"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
+
+	"github.com/pelicanplatform/pelican/param"
 )
 
 const (
-	DefaultSocketPath        = "~/.pelican/client-api.sock"
-	DefaultPidFile           = "~/.pelican/client-api.pid"
-	DefaultMaxConcurrentJobs = 5
-	DefaultShutdownTimeout   = 30 * time.Second
+	DefaultSocketPath      = "~/.pelican/client-api.sock"
+	DefaultPidFile         = "~/.pelican/client-api.pid"
+	DefaultShutdownTimeout = 30 * time.Second
 )
 
 // Server represents the client API server
@@ -126,9 +127,15 @@ func NewServer(config ServerConfig) (*Server, error) {
 	}
 
 	// Create transfer manager
+	// Use config value if provided, otherwise fall back to parameter
 	maxJobs := config.MaxConcurrentJobs
 	if maxJobs <= 0 {
-		maxJobs = DefaultMaxConcurrentJobs
+		providedValue := maxJobs
+		maxJobs = param.ClientAgent_MaxConcurrentJobs.GetInt()
+		if maxJobs <= 0 {
+			log.Warnf("Invalid max concurrent jobs configuration (provided=%d, param=%d), using default value of 5", providedValue, maxJobs)
+			maxJobs = 5 // Final fallback
+		}
 	}
 	transferManager := NewTransferManager(ctx, maxJobs, storeInstance)
 

client_agent/transfer_manager.go

@@ -28,6 +28,7 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"github.com/pelicanplatform/pelican/client"
+	"github.com/pelicanplatform/pelican/param"
 )
 
 // Transfer represents an individual file transfer
@@ -81,6 +82,16 @@ type TransferManager struct {
 func NewTransferManager(ctx context.Context, maxConcurrentJobs int, store StoreInterface) *TransferManager {
 	managerCtx, cancel := context.WithCancel(ctx)
 
+	// Use parameter value if maxConcurrentJobs is not positive
+	if maxConcurrentJobs <= 0 {
+		providedValue := maxConcurrentJobs
+		maxConcurrentJobs = param.ClientAgent_MaxConcurrentJobs.GetInt()
+		if maxConcurrentJobs <= 0 {
+			log.Warnf("Invalid max concurrent jobs configuration (provided=%d, param=%d), using default value of 5", providedValue, maxConcurrentJobs)
+			maxConcurrentJobs = 5 // Final fallback
+		}
+	}
+
 	tm := &TransferManager{
 		jobs:      make(map[string]*TransferJob),
 		transfers: make(map[string]*Transfer),

cmd/client_agent.go

@@ -233,8 +233,8 @@ func init() {
 		"Path to the PID file")
 
 	// Serve-specific flags
-	clientAgentServeCmd.Flags().IntVar(&clientAgentMaxJobs, "max-jobs", client_agent.DefaultMaxConcurrentJobs,
-		"Maximum number of concurrent transfer jobs")
+	clientAgentServeCmd.Flags().IntVar(&clientAgentMaxJobs, "max-jobs", 0,
+		"Maximum number of concurrent transfer jobs (default: uses ClientAgent.MaxConcurrentJobs parameter, or 5)")
 	clientAgentServeCmd.Flags().StringVar(&clientAgentDbPath, "database", "",
 		"Path to the SQLite database file for persistence (default: ~/.pelican/client-api.db)")
 

docs/parameters.yaml

@@ -693,17 +693,6 @@ type: filename
 default: $ConfigBase/client-api.db
 components: ["client"]
 ---
-name: ClientAgent.EnableAutoRecovery
-description: |+
-  A boolean indicating whether the client API server should automatically attempt
-  to recover incomplete jobs from the database on startup. When enabled, interrupted
-  jobs will be marked as failed and archived to history.
-
-  Disabling this may be useful for debugging or when manual recovery is preferred.
-type: bool
-default: true
-components: ["client"]
----
 name: ClientAgent.HistoryRetentionDays
 description: |+
   The number of days to retain completed job history in the database before
@@ -715,16 +704,15 @@ type: int
 default: 30
 components: ["client"]
 ---
-name: ClientAgent.MaxConnections
+name: ClientAgent.MaxConcurrentJobs
 description: |+
-  The maximum number of concurrent database connections allowed by the client API
-  server's SQLite connection pool. This limits resource usage but may impact
-  performance under high load.
+  The maximum number of concurrent transfer jobs that the client API server
+  can process simultaneously. This limits resource usage and prevents overwhelming
+  the system with too many parallel transfers.
 
-  Increase this value if you experience database lock contention or timeouts
-  with many concurrent API requests.
+  Set to 0 or negative to use the default value (5).
 type: int
-default: 10
+default: 5
 components: ["client"]
 ---
 name: ClientAgent.Socket
@@ -733,9 +721,7 @@ description: |+
   inter-process communication. Clients connect to this socket to submit transfer
   jobs and query their status.
 
-  If not specified, the default location is used based on the platform and user
-  permissions. This parameter allows overriding the default location for testing
-  or custom deployment scenarios.
+  If not specified, the default location is `~/.pelican/client-api.db`.
 type: filename
 default: ""
 components: ["client"]