From c1f73d616cee393403a4f7989a064f70a6e54091 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Mon, 3 Nov 2025 13:08:14 -0700 Subject: [PATCH 1/8] Refactor upload to allow uploading a subset of files --- cmd/src/snapshot_upload.go | 358 ++++++++++++++++++++++---------- cmd/src/snapshot_upload_test.go | 335 ++++++++++++++++++++++++++++++ 2 files changed, 585 insertions(+), 108 deletions(-) create mode 100644 cmd/src/snapshot_upload_test.go diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index 85e938f6e8..6e14b611a3 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -5,9 +5,10 @@ import ( "flag" "fmt" "io" - "io/fs" "os" "path" + "path/filepath" + "strings" "cloud.google.com/go/storage" "github.com/sourcegraph/conc/pool" @@ -18,168 +19,309 @@ import ( "github.com/sourcegraph/src-cli/internal/pgdump" ) +// GCP docs +// https://pkg.go.dev/cloud.google.com/go/storage#section-readme + const srcSnapshotDir = "./src-snapshot" var srcSnapshotSummaryPath = path.Join(srcSnapshotDir, "summary.json") -// https://pkg.go.dev/cloud.google.com/go/storage#section-readme +// Define types +type uploadArgs struct { + bucketName string + credentialsPath string + filterSQL bool + filesToUpload []string +} + +type uploadClients struct { + ctx context.Context + out *output.Output + storageClient *storage.Client +} + +// uploadFile represents a file registered for upload +type uploadFile struct { + file *os.File + stat os.FileInfo + filterSQL bool // Whether to filter incompatible SQL statements during upload, true for database files, false for summary file +} + func init() { usage := `'src snapshot upload' uploads instance snapshot contents generated by 'src snapshot databases' and 'src snapshot summary' to the designated bucket. USAGE - src snapshot upload -bucket=$BUCKET -credentials=$CREDENTIALS_FILE + src snapshot upload -bucket=$MIGRATION_BUCKET_NAME -credentials=./migration_private_key.json [-file=$FILE] BUCKET In general, a Google Cloud Storage bucket and relevant credentials will be provided by Sourcegraph when using this functionality to share a snapshot with Sourcegraph. + +FILE + Optional: Specify files to upload as a comma-delimited list. Valid values: summary, primary, codeintel, codeinsights. Default: All valid values. + Examples: -file=summary,primary or -file=codeintel ` + flagSet := flag.NewFlagSet("upload", flag.ExitOnError) bucketName := flagSet.String("bucket", "", "destination Cloud Storage bucket name") credentialsPath := flagSet.String("credentials", "", "JSON credentials file for Google Cloud service account") - trimExtensions := flagSet.Bool("trim-extensions", true, "trim EXTENSION statements from database dumps for import to Google Cloud SQL") + filterSQL := flagSet.Bool("filter-sql", true, "filter incompatible SQL statements from database dumps for import to Google Cloud SQL") + fileFilter := flagSet.String("file", "summary,primary,codeintel,codeinsights", "comma-delimited list of files to upload") + // Register this command with the parent 'src snapshot' command. + // The parent snapshot.go command runs all registered subcommands via snapshotCommands.run(). + // This self-registration pattern allows subcommands to automatically register themselves + // when their init() functions run, without requiring a central registry file. snapshotCommands = append(snapshotCommands, &command{ - flagSet: flagSet, - handler: func(args []string) error { - if err := flagSet.Parse(args); err != nil { - return err - } + flagSet: flagSet, + handler: snapshotUploadHandler(flagSet, bucketName, credentialsPath, filterSQL, fileFilter), + usageFunc: func() { fmt.Fprint(flag.CommandLine.Output(), usage) }, + }) +} - if *bucketName == "" { - return errors.New("-bucket required") - } - if *credentialsPath == "" { - return errors.New("-credentials required") - } +// Helper function to keep init() succinct +func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *string, filterSQL *bool, fileFilter *string) func([]string) error { + return func(args []string) error { + if err := flagSet.Parse(args); err != nil { + return err + } - out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) - ctx := context.Background() - c, err := storage.NewClient(ctx, option.WithCredentialsFile(*credentialsPath)) - if err != nil { - return errors.Wrap(err, "create Cloud Storage client") - } + // Validate and parse inputs + uploadArgs, err := validateUploadInputs(*bucketName, *credentialsPath, *fileFilter, *filterSQL) + if err != nil { + return err + } - type upload struct { - file *os.File - stat os.FileInfo - trimExtensions bool - } - var ( - uploads []upload // index aligned with progressBars - progressBars []output.ProgressBar // index aligned with uploads - ) + // Create clients + clients, err := createUploadClients(flagSet, uploadArgs.credentialsPath) + if err != nil { + return err + } + + // Open files and create progress bars + openedFiles, progressBars, err := openFilesAndCreateProgressBars(uploadArgs) + if err != nil { + return err + } - // Open snapshot summary - if f, err := os.Open(srcSnapshotSummaryPath); err != nil { + // Upload files to bucket + return uploadFilesToBucket(clients, uploadArgs, openedFiles, progressBars) + } +} + +// Validate user inputs +func validateUploadInputs(bucketName, credentialsPath, fileFilter string, filterSQL bool) (*uploadArgs, error) { + if bucketName == "" { + return nil, errors.New("-bucket required") + } + if credentialsPath == "" { + return nil, errors.New("-credentials required") + } + + filesToUpload, err := parseFileFilter(fileFilter) + if err != nil { + return nil, err + } + + return &uploadArgs{ + bucketName: bucketName, + credentialsPath: credentialsPath, + filterSQL: filterSQL, + filesToUpload: filesToUpload, + }, nil +} + +// Parse the --file arg values +func parseFileFilter(fileFilter string) ([]string, error) { + + validFiles := map[string]bool{ + "summary": true, + "primary": true, + "codeintel": true, + "codeinsights": true, + } + + if fileFilter == "" { + // Default: all files + return []string{"summary", "primary", "codeintel", "codeinsights"}, nil + } + + // Parse comma-delimited list + var filesToUpload []string + parts := strings.Split(fileFilter, ",") + for _, part := range parts { + + // Normalize: trim spaces and strip file extensions + normalized := strings.TrimSpace(part) + normalized = strings.TrimSuffix(normalized, ".json") + normalized = strings.TrimSuffix(normalized, ".sql") + + // Validate + if !validFiles[normalized] { + return nil, errors.Newf("invalid -file value %q. Valid values: summary[.json], primary[.sql], codeintel[.sql], codeinsights[.sql]", part) + } + + filesToUpload = append(filesToUpload, normalized) + } + + return filesToUpload, nil +} + +func createUploadClients(flagSet *flag.FlagSet, credentialsPath string) (*uploadClients, error) { + + out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) + ctx := context.Background() + client, err := storage.NewClient(ctx, option.WithCredentialsFile(credentialsPath)) + + if err != nil { + return nil, errors.Wrap(err, "create Cloud Storage client") + } + + // TODO: Does upload client need to be plural? + return &uploadClients{ + ctx: ctx, + out: out, + storageClient: client, + }, nil +} + +// openFilesAndCreateProgressBars opens selected snapshot files from disk and creates progress bars for UI display. +// Returns arrays of uploadFile and progress bars (aligned by index). +func openFilesAndCreateProgressBars(args *uploadArgs) ([]uploadFile, []output.ProgressBar, error) { + var ( + openedFiles []uploadFile // Files opened from disk, ready for upload (aligned with progressBars) + progressBars []output.ProgressBar // Progress bars for UI (aligned with openedFiles) + ) + + // addFile opens a file from disk and registers it for upload. + // It adds the file to the openedFiles array and creates a corresponding progress bar. + // For database dumps (!isSummary), SQL filtering is enabled based on args.filterSQL. + addFile := func(filePath string, isSummary bool) error { + + // Open the file + openFile, err := os.Open(filePath) + if err != nil { + if isSummary { return errors.Wrap(err, "failed to open snapshot summary - generate one with 'src snapshot summary'") - } else { - stat, err := f.Stat() - if err != nil { - return errors.Wrap(err, "get file size") - } - uploads = append(uploads, upload{ - file: f, - stat: stat, - trimExtensions: false, // not a database dump - }) - progressBars = append(progressBars, output.ProgressBar{ - Label: stat.Name(), - Max: float64(stat.Size()), - }) } + return errors.Wrap(err, "failed to open database dump - generate one with 'src snapshot databases'") + } - // Open database dumps - for _, o := range pgdump.Outputs(srcSnapshotDir, pgdump.Targets{}) { - if f, err := os.Open(o.Output); err != nil { - return errors.Wrap(err, "failed to database dump - generate one with 'src snapshot databases'") - } else { - stat, err := f.Stat() - if err != nil { - return errors.Wrap(err, "get file size") - } - uploads = append(uploads, upload{ - file: f, - stat: stat, - trimExtensions: *trimExtensions, - }) - progressBars = append(progressBars, output.ProgressBar{ - Label: stat.Name(), - Max: float64(stat.Size()), - }) - } - } + // Get file metadata (name, size) + stat, err := openFile.Stat() + if err != nil { + return errors.Wrap(err, "get file size") + } + + // Register file for upload + openedFiles = append(openedFiles, uploadFile{ + file: openFile, + stat: stat, + filterSQL: !isSummary && args.filterSQL, // Only filter SQL for database dumps + }) + + // Create progress bar for this file + progressBars = append(progressBars, output.ProgressBar{ + Label: stat.Name(), + Max: float64(stat.Size()), + }) + return nil + } - // Start uploads - progress := out.Progress(progressBars, nil) - progress.WriteLine(output.Emoji(output.EmojiHourglass, "Starting uploads...")) - bucket := c.Bucket(*bucketName) - g := pool.New().WithErrors().WithContext(ctx) - for i, u := range uploads { - i := i - u := u - g.Go(func(ctx context.Context) error { - progressFn := func(p int64) { progress.SetValue(i, float64(p)) } - - if err := copyDumpToBucket(ctx, u.file, u.stat, bucket, progressFn, u.trimExtensions); err != nil { - return errors.Wrap(err, u.stat.Name()) - } - - return nil - }) + // Open files based on user's selection (via --file arg) + // Iterate through the user's selected files and open each one + for _, selectedFile := range args.filesToUpload { + if selectedFile == "summary" { + // Open summary.json + if err := addFile(srcSnapshotSummaryPath, true); err != nil { + return nil, nil, err } + } else { + // Open database dump file (primary.sql, codeintel.sql, or codeinsights.sql) + dbFilePath := filepath.Join(srcSnapshotDir, selectedFile+".sql") + if err := addFile(dbFilePath, false); err != nil { + return nil, nil, err + } + } + } + + return openedFiles, progressBars, nil +} - // Finalize - errs := g.Wait() - progress.Complete() - if errs != nil { - out.WriteLine(output.Line(output.EmojiFailure, output.StyleFailure, "Some snapshot contents failed to upload.")) - return errs +// uploadFilesToBucket uploads the prepared files to Google Cloud Storage bucket. +// Uploads are performed in parallel with progress tracking. +func uploadFilesToBucket(clients *uploadClients, args *uploadArgs, openedFiles []uploadFile, progressBars []output.ProgressBar) error { + // Start uploads with progress tracking + progress := clients.out.Progress(progressBars, nil) + progress.WriteLine(output.Emoji(output.EmojiHourglass, "Starting uploads...")) + bucket := clients.storageClient.Bucket(args.bucketName) + uploadPool := pool.New().WithErrors().WithContext(clients.ctx) + + // Upload each file in parallel + for fileIndex, openedFile := range openedFiles { + fileIndex := fileIndex + openedFile := openedFile + uploadPool.Go(func(ctx context.Context) error { + progressFn := func(bytesWritten int64) { progress.SetValue(fileIndex, float64(bytesWritten)) } + + if err := streamFileToBucket(ctx, &openedFile, bucket, progressFn); err != nil { + return errors.Wrap(err, openedFile.stat.Name()) } - out.WriteLine(output.Emoji(output.EmojiSuccess, "Summary contents uploaded!")) return nil - }, - usageFunc: func() { fmt.Fprint(flag.CommandLine.Output(), usage) }, - }) + }) + } + + // Wait for all uploads to complete + errs := uploadPool.Wait() + progress.Complete() + if errs != nil { + clients.out.WriteLine(output.Line(output.EmojiFailure, output.StyleFailure, "Some snapshot contents failed to upload.")) + return errs + } + + clients.out.WriteLine(output.Emoji(output.EmojiSuccess, "Summary contents uploaded!")) + return nil } -func copyDumpToBucket(ctx context.Context, src io.ReadSeeker, stat fs.FileInfo, dst *storage.BucketHandle, progressFn func(int64), trimExtensions bool) error { - // Set up object to write to - object := dst.Object(stat.Name()).NewWriter(ctx) - object.ProgressFunc = progressFn - defer object.Close() +func streamFileToBucket(ctx context.Context, file *uploadFile, bucket *storage.BucketHandle, progressFn func(int64)) error { + + // Set up GCS writer for the destination file + writer := bucket.Object(file.stat.Name()).NewWriter(ctx) + writer.ProgressFunc = progressFn + defer writer.Close() // To assert against actual file size - var totalWritten int64 + var totalBytesWritten int64 // Do a partial copy, that filters out incompatible statements - if trimExtensions { - written, err := pgdump.FilterInvalidLines(object, src, progressFn) + if file.filterSQL { + bytesWritten, err := pgdump.FilterInvalidLines(writer, file.file, progressFn) if err != nil { return errors.Wrap(err, "filter out incompatible statements and upload") } - totalWritten += written + totalBytesWritten += bytesWritten } // io.Copy is the best way to copy from a reader to writer in Go, // storage.Writer has its own chunking mechanisms internally. // io.Reader is stateful, so this copy will just continue from where FilterInvalidLines left off, if used - written, err := io.Copy(object, src) + bytesWritten, err := io.Copy(writer, file.file) if err != nil { return errors.Wrap(err, "upload") } - totalWritten += written + totalBytesWritten += bytesWritten // Progress is not called on completion of io.Copy, // so we call it manually after to update our pretty progress bars. - progressFn(written) + progressFn(bytesWritten) // Validate we have sent all data. // FilterInvalidLines may add some bytes, so the check is not a strict equality. - size := stat.Size() - if totalWritten < size { + fileSize := file.stat.Size() + if totalBytesWritten < fileSize { return errors.Newf("expected to write %d bytes, but actually wrote %d bytes (diff: %d bytes)", - size, totalWritten, totalWritten-size) + fileSize, totalBytesWritten, totalBytesWritten-fileSize) } return nil diff --git a/cmd/src/snapshot_upload_test.go b/cmd/src/snapshot_upload_test.go new file mode 100644 index 0000000000..80f13bf43d --- /dev/null +++ b/cmd/src/snapshot_upload_test.go @@ -0,0 +1,335 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/sourcegraph/src-cli/internal/pgdump" +) + +func setupSnapshotFiles(t *testing.T) string { + t.Helper() + dir := t.TempDir() + + // Create the snapshot directory structure + snapshotDir := filepath.Join(dir, "src-snapshot") + err := os.Mkdir(snapshotDir, 0755) + require.NoError(t, err) + + // Create summary.json + summaryPath := filepath.Join(snapshotDir, "summary.json") + err = os.WriteFile(summaryPath, []byte(`{"version": "test"}`), 0644) + require.NoError(t, err) + + // Create database dump files + for _, output := range pgdump.Outputs(snapshotDir, pgdump.Targets{}) { + err = os.WriteFile(output.Output, []byte("-- test SQL dump"), 0644) + require.NoError(t, err) + } + + return snapshotDir +} + +func TestFileFilterValidation(t *testing.T) { + tests := []struct { + name string + fileFlag string + wantError bool + }{ + { + name: "valid: summary", + fileFlag: "summary", + wantError: false, + }, + { + name: "valid: primary", + fileFlag: "primary", + wantError: false, + }, + { + name: "valid: codeintel", + fileFlag: "codeintel", + wantError: false, + }, + { + name: "valid: codeinsights", + fileFlag: "codeinsights", + wantError: false, + }, + { + name: "valid: empty (all files)", + fileFlag: "", + wantError: false, + }, + { + name: "valid: comma-delimited", + fileFlag: "summary,primary", + wantError: false, + }, + { + name: "valid: comma-delimited with spaces", + fileFlag: "summary, primary, codeintel", + wantError: false, + }, + { + name: "valid: with .sql extension", + fileFlag: "primary.sql", + wantError: false, + }, + { + name: "valid: with .json extension", + fileFlag: "summary.json", + wantError: false, + }, + { + name: "valid: mixed extensions", + fileFlag: "summary.json,primary.sql", + wantError: false, + }, + { + name: "invalid: unknown file", + fileFlag: "unknown", + wantError: true, + }, + { + name: "invalid: typo", + fileFlag: "primry", + wantError: true, + }, + { + name: "invalid: one valid, one invalid", + fileFlag: "summary,invalid", + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the validation logic + validFiles := map[string]bool{ + "summary": true, + "primary": true, + "codeintel": true, + "codeinsights": true, + } + + var hasError bool + if tt.fileFlag == "" { + // Empty is valid (defaults to all files) + hasError = false + } else { + parts := strings.Split(tt.fileFlag, ",") + for _, part := range parts { + normalized := strings.TrimSpace(part) + normalized = strings.TrimSuffix(normalized, ".json") + normalized = strings.TrimSuffix(normalized, ".sql") + + if !validFiles[normalized] { + hasError = true + break + } + } + } + + if tt.wantError { + require.True(t, hasError, "expected invalid file flag to be rejected") + } else { + require.False(t, hasError, "expected valid file flag to be accepted") + } + }) + } +} + +func TestFileSelection(t *testing.T) { + snapshotDir := setupSnapshotFiles(t) + + tests := []struct { + name string + fileFilter string + expectedFiles []string + }{ + { + name: "no filter - all files", + fileFilter: "", + expectedFiles: []string{ + "summary.json", + "primary.sql", + "codeintel.sql", + "codeinsights.sql", + }, + }, + { + name: "summary only", + fileFilter: "summary", + expectedFiles: []string{ + "summary.json", + }, + }, + { + name: "primary only", + fileFilter: "primary", + expectedFiles: []string{ + "primary.sql", + }, + }, + { + name: "codeintel only", + fileFilter: "codeintel", + expectedFiles: []string{ + "codeintel.sql", + }, + }, + { + name: "codeinsights only", + fileFilter: "codeinsights", + expectedFiles: []string{ + "codeinsights.sql", + }, + }, + { + name: "comma-delimited: summary and primary", + fileFilter: "summary,primary", + expectedFiles: []string{ + "summary.json", + "primary.sql", + }, + }, + { + name: "comma-delimited: all database files", + fileFilter: "primary,codeintel,codeinsights", + expectedFiles: []string{ + "primary.sql", + "codeintel.sql", + "codeinsights.sql", + }, + }, + { + name: "comma-delimited with extensions", + fileFilter: "summary.json,primary.sql", + expectedFiles: []string{ + "summary.json", + "primary.sql", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Parse file filter into list + var filesToUpload []string + if tt.fileFilter == "" { + filesToUpload = []string{"summary", "primary", "codeintel", "codeinsights"} + } else { + parts := strings.Split(tt.fileFilter, ",") + for _, part := range parts { + normalized := strings.TrimSpace(part) + normalized = strings.TrimSuffix(normalized, ".json") + normalized = strings.TrimSuffix(normalized, ".sql") + filesToUpload = append(filesToUpload, normalized) + } + } + + // Helper to check if a file should be uploaded + shouldUpload := func(fileType string) bool { + for _, f := range filesToUpload { + if f == fileType { + return true + } + } + return false + } + + var selectedFiles []string + + // Simulate the file selection logic from snapshot_upload.go + if shouldUpload("summary") { + selectedFiles = append(selectedFiles, "summary.json") + } + + for _, o := range pgdump.Outputs(snapshotDir, pgdump.Targets{}) { + fileName := filepath.Base(o.Output) + fileType := fileName[:len(fileName)-4] // Remove .sql extension + + if shouldUpload(fileType) { + selectedFiles = append(selectedFiles, fileName) + } + } + + require.Equal(t, tt.expectedFiles, selectedFiles, "selected files should match expected") + }) + } +} + +func TestFilterSQLBehavior(t *testing.T) { + tests := []struct { + name string + isSummary bool + filterSQLFlag bool + expectedFilter bool + }{ + { + name: "summary file - filterSQL should be false", + isSummary: true, + filterSQLFlag: true, + expectedFilter: false, + }, + { + name: "database dump - filterSQL flag true", + isSummary: false, + filterSQLFlag: true, + expectedFilter: true, + }, + { + name: "database dump - filterSQL flag false", + isSummary: false, + filterSQLFlag: false, + expectedFilter: false, + }, + { + name: "summary file - filterSQL flag false (should still be false)", + isSummary: true, + filterSQLFlag: false, + expectedFilter: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Simulate the filterSQL logic from snapshot_upload.go + actualFilter := !tt.isSummary && tt.filterSQLFlag + + require.Equal(t, tt.expectedFilter, actualFilter, "filterSQL should be set correctly") + }) + } +} + +func TestDatabaseOutputs(t *testing.T) { + snapshotDir := setupSnapshotFiles(t) + + outputs := pgdump.Outputs(snapshotDir, pgdump.Targets{}) + + // Should have exactly 3 database files + require.Len(t, outputs, 3, "should have 3 database outputs") + + expectedFiles := map[string]bool{ + "primary.sql": false, + "codeintel.sql": false, + "codeinsights.sql": false, + } + + for _, output := range outputs { + fileName := filepath.Base(output.Output) + _, exists := expectedFiles[fileName] + require.True(t, exists, "unexpected file: %s", fileName) + expectedFiles[fileName] = true + } + + // Verify all expected files were found + for fileName, found := range expectedFiles { + require.True(t, found, "expected file not found: %s", fileName) + } +} From 187849f909a0f9b8d1a3ee0c56a50cf9b678cf9f Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Tue, 4 Nov 2025 02:02:00 -0500 Subject: [PATCH 2/8] Fix variable names --- cmd/src/snapshot_upload.go | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index 6e14b611a3..0fd56a9ad3 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -19,9 +19,6 @@ import ( "github.com/sourcegraph/src-cli/internal/pgdump" ) -// GCP docs -// https://pkg.go.dev/cloud.google.com/go/storage#section-readme - const srcSnapshotDir = "./src-snapshot" var srcSnapshotSummaryPath = path.Join(srcSnapshotDir, "summary.json") @@ -34,7 +31,7 @@ type uploadArgs struct { filesToUpload []string } -type uploadClients struct { +type gcsClient struct { ctx context.Context out *output.Output storageClient *storage.Client @@ -92,8 +89,8 @@ func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *s return err } - // Create clients - clients, err := createUploadClients(flagSet, uploadArgs.credentialsPath) + // Create client + client, err := createGcsClient(flagSet, uploadArgs.credentialsPath) if err != nil { return err } @@ -105,11 +102,11 @@ func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *s } // Upload files to bucket - return uploadFilesToBucket(clients, uploadArgs, openedFiles, progressBars) + return uploadFilesToBucket(client, uploadArgs, openedFiles, progressBars) } } -// Validate user inputs +// Validate user inputs, and convert them to an object of type uploadArgs func validateUploadInputs(bucketName, credentialsPath, fileFilter string, filterSQL bool) (*uploadArgs, error) { if bucketName == "" { return nil, errors.New("-bucket required") @@ -131,7 +128,7 @@ func validateUploadInputs(bucketName, credentialsPath, fileFilter string, filter }, nil } -// Parse the --file arg values +// Parse the --file arg values, and return a list of strings of the files to upload func parseFileFilter(fileFilter string) ([]string, error) { validFiles := map[string]bool{ @@ -167,18 +164,18 @@ func parseFileFilter(fileFilter string) ([]string, error) { return filesToUpload, nil } -func createUploadClients(flagSet *flag.FlagSet, credentialsPath string) (*uploadClients, error) { +func createGcsClient(flagSet *flag.FlagSet, credentialsPath string) (*gcsClient, error) { out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) ctx := context.Background() + // https://pkg.go.dev/cloud.google.com/go/storage#section-readme client, err := storage.NewClient(ctx, option.WithCredentialsFile(credentialsPath)) if err != nil { - return nil, errors.Wrap(err, "create Cloud Storage client") + return nil, errors.Wrap(err, "create Google Cloud Storage client") } - // TODO: Does upload client need to be plural? - return &uploadClients{ + return &gcsClient{ ctx: ctx, out: out, storageClient: client, @@ -250,12 +247,12 @@ func openFilesAndCreateProgressBars(args *uploadArgs) ([]uploadFile, []output.Pr // uploadFilesToBucket uploads the prepared files to Google Cloud Storage bucket. // Uploads are performed in parallel with progress tracking. -func uploadFilesToBucket(clients *uploadClients, args *uploadArgs, openedFiles []uploadFile, progressBars []output.ProgressBar) error { +func uploadFilesToBucket(client *gcsClient, args *uploadArgs, openedFiles []uploadFile, progressBars []output.ProgressBar) error { // Start uploads with progress tracking - progress := clients.out.Progress(progressBars, nil) + progress := client.out.Progress(progressBars, nil) progress.WriteLine(output.Emoji(output.EmojiHourglass, "Starting uploads...")) - bucket := clients.storageClient.Bucket(args.bucketName) - uploadPool := pool.New().WithErrors().WithContext(clients.ctx) + bucket := client.storageClient.Bucket(args.bucketName) + uploadPool := pool.New().WithErrors().WithContext(client.ctx) // Upload each file in parallel for fileIndex, openedFile := range openedFiles { @@ -276,11 +273,11 @@ func uploadFilesToBucket(clients *uploadClients, args *uploadArgs, openedFiles [ errs := uploadPool.Wait() progress.Complete() if errs != nil { - clients.out.WriteLine(output.Line(output.EmojiFailure, output.StyleFailure, "Some snapshot contents failed to upload.")) + client.out.WriteLine(output.Line(output.EmojiFailure, output.StyleFailure, "Some snapshot contents failed to upload.")) return errs } - clients.out.WriteLine(output.Emoji(output.EmojiSuccess, "Summary contents uploaded!")) + client.out.WriteLine(output.Emoji(output.EmojiSuccess, "Summary contents uploaded!")) return nil } From bb232c81229029d088bfabbeafcc8bb1c12b2cca Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Tue, 4 Nov 2025 03:47:43 -0500 Subject: [PATCH 3/8] Improved readability and maintainability --- cmd/src/snapshot_upload.go | 109 ++++++++++++---------- cmd/src/snapshot_upload_test.go | 159 +++++++++++--------------------- 2 files changed, 115 insertions(+), 153 deletions(-) diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index 0fd56a9ad3..a5f05a6077 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -6,8 +6,8 @@ import ( "fmt" "io" "os" - "path" "path/filepath" + "slices" "strings" "cloud.google.com/go/storage" @@ -19,9 +19,20 @@ import ( "github.com/sourcegraph/src-cli/internal/pgdump" ) +// Package-level variables const srcSnapshotDir = "./src-snapshot" -var srcSnapshotSummaryPath = path.Join(srcSnapshotDir, "summary.json") +// summaryFile on its own, as it gets handled a little differently +const summaryFile = "summary.json" +var srcSnapshotSummaryPath = filepath.Join(srcSnapshotDir, summaryFile) + +// listOfValidFiles defines the valid snapshot filenames (with extensions) that can be uploaded +var listOfValidFiles = []string{ + "codeinsights.sql", + "codeintel.sql", + "pgsql.sql", + summaryFile, +} // Define types type uploadArgs struct { @@ -31,13 +42,14 @@ type uploadArgs struct { filesToUpload []string } +// Google Cloud Storage upload client type gcsClient struct { ctx context.Context out *output.Output storageClient *storage.Client } -// uploadFile represents a file registered for upload +// uploadFile represents a file opened for upload type uploadFile struct { file *os.File stat os.FileInfo @@ -46,7 +58,7 @@ type uploadFile struct { func init() { - usage := `'src snapshot upload' uploads instance snapshot contents generated by 'src snapshot databases' and 'src snapshot summary' to the designated bucket. + usage := fmt.Sprintf(`'src snapshot upload' uploads instance snapshot contents generated by 'src snapshot databases' and 'src snapshot summary' to the designated bucket. USAGE src snapshot upload -bucket=$MIGRATION_BUCKET_NAME -credentials=./migration_private_key.json [-file=$FILE] @@ -55,15 +67,15 @@ BUCKET In general, a Google Cloud Storage bucket and relevant credentials will be provided by Sourcegraph when using this functionality to share a snapshot with Sourcegraph. FILE - Optional: Specify files to upload as a comma-delimited list. Valid values: summary, primary, codeintel, codeinsights. Default: All valid values. - Examples: -file=summary,primary or -file=codeintel -` + Optional: Specify files to upload as a comma-delimited list (with file extensions). Valid values: %s. Default: All files. + Examples: -file=summary.json,pgsql.sql or -file=codeintel.sql +`, strings.Join(listOfValidFiles, ", ")) flagSet := flag.NewFlagSet("upload", flag.ExitOnError) bucketName := flagSet.String("bucket", "", "destination Cloud Storage bucket name") credentialsPath := flagSet.String("credentials", "", "JSON credentials file for Google Cloud service account") + fileFilter := flagSet.String("file", strings.Join(listOfValidFiles, ","), "comma-delimited list of files to upload") filterSQL := flagSet.Bool("filter-sql", true, "filter incompatible SQL statements from database dumps for import to Google Cloud SQL") - fileFilter := flagSet.String("file", "summary,primary,codeintel,codeinsights", "comma-delimited list of files to upload") // Register this command with the parent 'src snapshot' command. // The parent snapshot.go command runs all registered subcommands via snapshotCommands.run(). @@ -76,14 +88,14 @@ FILE }) } -// Helper function to keep init() succinct +// Handler function to keep init() succinct func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *string, filterSQL *bool, fileFilter *string) func([]string) error { return func(args []string) error { if err := flagSet.Parse(args); err != nil { return err } - // Validate and parse inputs + // Validate and parse inputs into an uploadArgs-type object uploadArgs, err := validateUploadInputs(*bucketName, *credentialsPath, *fileFilter, *filterSQL) if err != nil { return err @@ -131,43 +143,41 @@ func validateUploadInputs(bucketName, credentialsPath, fileFilter string, filter // Parse the --file arg values, and return a list of strings of the files to upload func parseFileFilter(fileFilter string) ([]string, error) { - validFiles := map[string]bool{ - "summary": true, - "primary": true, - "codeintel": true, - "codeinsights": true, - } - + // Default: all files if fileFilter == "" { - // Default: all files - return []string{"summary", "primary", "codeintel", "codeinsights"}, nil + return listOfValidFiles, nil } - // Parse comma-delimited list var filesToUpload []string - parts := strings.Split(fileFilter, ",") - for _, part := range parts { - // Normalize: trim spaces and strip file extensions - normalized := strings.TrimSpace(part) - normalized = strings.TrimSuffix(normalized, ".json") - normalized = strings.TrimSuffix(normalized, ".sql") + // Parse comma-delimited list + for _, part := range strings.Split(fileFilter, ",") { - // Validate - if !validFiles[normalized] { - return nil, errors.Newf("invalid -file value %q. Valid values: summary[.json], primary[.sql], codeintel[.sql], codeinsights[.sql]", part) + // Trim whitespace + filename := strings.TrimSpace(part) + + // Validate against list of valid files + if !slices.Contains(listOfValidFiles, filename) { + return nil, errors.Newf("invalid -file value %q. Valid values: %s", part, strings.Join(listOfValidFiles, ", ")) } - filesToUpload = append(filesToUpload, normalized) + filesToUpload = append(filesToUpload, filename) } + // Sort files alphabetically for consistent ordering + slices.Sort(filesToUpload) + + // Remove duplicates (works on sorted slices by removing adjacent duplicates) + filesToUpload = slices.Compact(filesToUpload) + return filesToUpload, nil } func createGcsClient(flagSet *flag.FlagSet, credentialsPath string) (*gcsClient, error) { - out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) ctx := context.Background() + out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) + // https://pkg.go.dev/cloud.google.com/go/storage#section-readme client, err := storage.NewClient(ctx, option.WithCredentialsFile(credentialsPath)) @@ -186,22 +196,25 @@ func createGcsClient(flagSet *flag.FlagSet, credentialsPath string) (*gcsClient, // Returns arrays of uploadFile and progress bars (aligned by index). func openFilesAndCreateProgressBars(args *uploadArgs) ([]uploadFile, []output.ProgressBar, error) { var ( - openedFiles []uploadFile // Files opened from disk, ready for upload (aligned with progressBars) + openedFiles []uploadFile // Files opened from disk, ready for upload (aligned with progressBars) progressBars []output.ProgressBar // Progress bars for UI (aligned with openedFiles) ) // addFile opens a file from disk and registers it for upload. // It adds the file to the openedFiles array and creates a corresponding progress bar. // For database dumps (!isSummary), SQL filtering is enabled based on args.filterSQL. - addFile := func(filePath string, isSummary bool) error { + addFile := func(filePath string) error { + + isSummary := strings.HasSuffix(filePath, summaryFile) // Open the file openFile, err := os.Open(filePath) + if err != nil { if isSummary { - return errors.Wrap(err, "failed to open snapshot summary - generate one with 'src snapshot summary'") + return errors.Wrap(err, fmt.Sprintf("failed to open snapshot summary %s - Please generate it with 'src snapshot summary'", filePath)) } - return errors.Wrap(err, "failed to open database dump - generate one with 'src snapshot databases'") + return errors.Wrap(err, fmt.Sprintf("failed to open database dump %s - Please generate them with 'src snapshot databases'", filePath)) } // Get file metadata (name, size) @@ -228,17 +241,12 @@ func openFilesAndCreateProgressBars(args *uploadArgs) ([]uploadFile, []output.Pr // Open files based on user's selection (via --file arg) // Iterate through the user's selected files and open each one for _, selectedFile := range args.filesToUpload { - if selectedFile == "summary" { - // Open summary.json - if err := addFile(srcSnapshotSummaryPath, true); err != nil { - return nil, nil, err - } - } else { - // Open database dump file (primary.sql, codeintel.sql, or codeinsights.sql) - dbFilePath := filepath.Join(srcSnapshotDir, selectedFile+".sql") - if err := addFile(dbFilePath, false); err != nil { - return nil, nil, err - } + + // Construct full file path + filePath := filepath.Join(srcSnapshotDir, selectedFile) + + if err := addFile(filePath); err != nil { + return nil, nil, err } } @@ -248,6 +256,7 @@ func openFilesAndCreateProgressBars(args *uploadArgs) ([]uploadFile, []output.Pr // uploadFilesToBucket uploads the prepared files to Google Cloud Storage bucket. // Uploads are performed in parallel with progress tracking. func uploadFilesToBucket(client *gcsClient, args *uploadArgs, openedFiles []uploadFile, progressBars []output.ProgressBar) error { + // Start uploads with progress tracking progress := client.out.Progress(progressBars, nil) progress.WriteLine(output.Emoji(output.EmojiHourglass, "Starting uploads...")) @@ -256,8 +265,10 @@ func uploadFilesToBucket(client *gcsClient, args *uploadArgs, openedFiles []uplo // Upload each file in parallel for fileIndex, openedFile := range openedFiles { + fileIndex := fileIndex openedFile := openedFile + uploadPool.Go(func(ctx context.Context) error { progressFn := func(bytesWritten int64) { progress.SetValue(fileIndex, float64(bytesWritten)) } @@ -273,11 +284,11 @@ func uploadFilesToBucket(client *gcsClient, args *uploadArgs, openedFiles []uplo errs := uploadPool.Wait() progress.Complete() if errs != nil { - client.out.WriteLine(output.Line(output.EmojiFailure, output.StyleFailure, "Some snapshot contents failed to upload.")) + client.out.WriteLine(output.Line(output.EmojiFailure, output.StyleFailure, "Some file(s) failed to upload.")) return errs } - client.out.WriteLine(output.Emoji(output.EmojiSuccess, "Summary contents uploaded!")) + client.out.WriteLine(output.Emoji(output.EmojiSuccess, "File(s) uploaded successfully!")) return nil } @@ -291,7 +302,7 @@ func streamFileToBucket(ctx context.Context, file *uploadFile, bucket *storage.B // To assert against actual file size var totalBytesWritten int64 - // Do a partial copy, that filters out incompatible statements + // Start a partial copy, that filters out incompatible statements if file.filterSQL { bytesWritten, err := pgdump.FilterInvalidLines(writer, file.file, progressFn) if err != nil { diff --git a/cmd/src/snapshot_upload_test.go b/cmd/src/snapshot_upload_test.go index 80f13bf43d..568e582111 100644 --- a/cmd/src/snapshot_upload_test.go +++ b/cmd/src/snapshot_upload_test.go @@ -3,6 +3,7 @@ package main import ( "os" "path/filepath" + "slices" "strings" "testing" @@ -14,23 +15,23 @@ import ( func setupSnapshotFiles(t *testing.T) string { t.Helper() dir := t.TempDir() - + // Create the snapshot directory structure snapshotDir := filepath.Join(dir, "src-snapshot") err := os.Mkdir(snapshotDir, 0755) require.NoError(t, err) - + // Create summary.json summaryPath := filepath.Join(snapshotDir, "summary.json") err = os.WriteFile(summaryPath, []byte(`{"version": "test"}`), 0644) require.NoError(t, err) - + // Create database dump files for _, output := range pgdump.Outputs(snapshotDir, pgdump.Targets{}) { err = os.WriteFile(output.Output, []byte("-- test SQL dump"), 0644) require.NoError(t, err) } - + return snapshotDir } @@ -41,23 +42,23 @@ func TestFileFilterValidation(t *testing.T) { wantError bool }{ { - name: "valid: summary", - fileFlag: "summary", + name: "valid: summary.json", + fileFlag: "summary.json", wantError: false, }, { - name: "valid: primary", - fileFlag: "primary", + name: "valid: pgsql.sql", + fileFlag: "pgsql.sql", wantError: false, }, { - name: "valid: codeintel", - fileFlag: "codeintel", + name: "valid: codeintel.sql", + fileFlag: "codeintel.sql", wantError: false, }, { - name: "valid: codeinsights", - fileFlag: "codeinsights", + name: "valid: codeinsights.sql", + fileFlag: "codeinsights.sql", wantError: false, }, { @@ -67,27 +68,12 @@ func TestFileFilterValidation(t *testing.T) { }, { name: "valid: comma-delimited", - fileFlag: "summary,primary", + fileFlag: "summary.json,pgsql.sql", wantError: false, }, { name: "valid: comma-delimited with spaces", - fileFlag: "summary, primary, codeintel", - wantError: false, - }, - { - name: "valid: with .sql extension", - fileFlag: "primary.sql", - wantError: false, - }, - { - name: "valid: with .json extension", - fileFlag: "summary.json", - wantError: false, - }, - { - name: "valid: mixed extensions", - fileFlag: "summary.json,primary.sql", + fileFlag: "summary.json, pgsql.sql, codeintel.sql", wantError: false, }, { @@ -102,21 +88,14 @@ func TestFileFilterValidation(t *testing.T) { }, { name: "invalid: one valid, one invalid", - fileFlag: "summary,invalid", + fileFlag: "summary.json,invalid", wantError: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Test the validation logic - validFiles := map[string]bool{ - "summary": true, - "primary": true, - "codeintel": true, - "codeinsights": true, - } - + // Test the validation logic using the shared listOfValidFiles var hasError bool if tt.fileFlag == "" { // Empty is valid (defaults to all files) @@ -124,17 +103,15 @@ func TestFileFilterValidation(t *testing.T) { } else { parts := strings.Split(tt.fileFlag, ",") for _, part := range parts { - normalized := strings.TrimSpace(part) - normalized = strings.TrimSuffix(normalized, ".json") - normalized = strings.TrimSuffix(normalized, ".sql") - - if !validFiles[normalized] { + filename := strings.TrimSpace(part) + + if !slices.Contains(listOfValidFiles, filename) { hasError = true break } } } - + if tt.wantError { require.True(t, hasError, "expected invalid file flag to be rejected") } else { @@ -146,7 +123,7 @@ func TestFileFilterValidation(t *testing.T) { func TestFileSelection(t *testing.T) { snapshotDir := setupSnapshotFiles(t) - + tests := []struct { name string fileFilter string @@ -156,109 +133,83 @@ func TestFileSelection(t *testing.T) { name: "no filter - all files", fileFilter: "", expectedFiles: []string{ - "summary.json", - "primary.sql", - "codeintel.sql", "codeinsights.sql", + "codeintel.sql", + "pgsql.sql", + "summary.json", }, }, { name: "summary only", - fileFilter: "summary", + fileFilter: "summary.json", expectedFiles: []string{ "summary.json", }, }, { - name: "primary only", - fileFilter: "primary", + name: "pgsql only", + fileFilter: "pgsql.sql", expectedFiles: []string{ - "primary.sql", + "pgsql.sql", }, }, { name: "codeintel only", - fileFilter: "codeintel", + fileFilter: "codeintel.sql", expectedFiles: []string{ "codeintel.sql", }, }, { name: "codeinsights only", - fileFilter: "codeinsights", + fileFilter: "codeinsights.sql", expectedFiles: []string{ "codeinsights.sql", }, }, { - name: "comma-delimited: summary and primary", - fileFilter: "summary,primary", + name: "comma-delimited: summary and pgsql", + fileFilter: "summary.json,pgsql.sql", expectedFiles: []string{ "summary.json", - "primary.sql", + "pgsql.sql", }, }, { name: "comma-delimited: all database files", - fileFilter: "primary,codeintel,codeinsights", + fileFilter: "pgsql.sql,codeintel.sql,codeinsights.sql", expectedFiles: []string{ - "primary.sql", + "pgsql.sql", "codeintel.sql", "codeinsights.sql", }, }, - { - name: "comma-delimited with extensions", - fileFilter: "summary.json,primary.sql", - expectedFiles: []string{ - "summary.json", - "primary.sql", - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Parse file filter into list + // Parse file filter into list (mimicking parseFileFilter logic) var filesToUpload []string if tt.fileFilter == "" { - filesToUpload = []string{"summary", "primary", "codeintel", "codeinsights"} + filesToUpload = listOfValidFiles } else { parts := strings.Split(tt.fileFilter, ",") for _, part := range parts { - normalized := strings.TrimSpace(part) - normalized = strings.TrimSuffix(normalized, ".json") - normalized = strings.TrimSuffix(normalized, ".sql") - filesToUpload = append(filesToUpload, normalized) - } - } - - // Helper to check if a file should be uploaded - shouldUpload := func(fileType string) bool { - for _, f := range filesToUpload { - if f == fileType { - return true - } + filename := strings.TrimSpace(part) + filesToUpload = append(filesToUpload, filename) } - return false } - + + // Simulate the file opening logic from openFilesAndCreateProgressBars var selectedFiles []string - - // Simulate the file selection logic from snapshot_upload.go - if shouldUpload("summary") { - selectedFiles = append(selectedFiles, "summary.json") - } - - for _, o := range pgdump.Outputs(snapshotDir, pgdump.Targets{}) { - fileName := filepath.Base(o.Output) - fileType := fileName[:len(fileName)-4] // Remove .sql extension - - if shouldUpload(fileType) { - selectedFiles = append(selectedFiles, fileName) + for _, selectedFile := range filesToUpload { + // Construct path and check if file matches + filePath := filepath.Join(snapshotDir, selectedFile) + if _, err := os.Stat(filePath); err == nil { + selectedFiles = append(selectedFiles, selectedFile) } } - + require.Equal(t, tt.expectedFiles, selectedFiles, "selected files should match expected") }) } @@ -301,7 +252,7 @@ func TestFilterSQLBehavior(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Simulate the filterSQL logic from snapshot_upload.go actualFilter := !tt.isSummary && tt.filterSQLFlag - + require.Equal(t, tt.expectedFilter, actualFilter, "filterSQL should be set correctly") }) } @@ -309,25 +260,25 @@ func TestFilterSQLBehavior(t *testing.T) { func TestDatabaseOutputs(t *testing.T) { snapshotDir := setupSnapshotFiles(t) - + outputs := pgdump.Outputs(snapshotDir, pgdump.Targets{}) - + // Should have exactly 3 database files require.Len(t, outputs, 3, "should have 3 database outputs") - + expectedFiles := map[string]bool{ - "primary.sql": false, + "pgsql.sql": false, "codeintel.sql": false, "codeinsights.sql": false, } - + for _, output := range outputs { fileName := filepath.Base(output.Output) _, exists := expectedFiles[fileName] require.True(t, exists, "unexpected file: %s", fileName) expectedFiles[fileName] = true } - + // Verify all expected files were found for fileName, found := range expectedFiles { require.True(t, found, "expected file not found: %s", fileName) From d40289d9d7b294c52d0551cf3868611c9a0b7367 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Tue, 4 Nov 2025 03:54:12 -0500 Subject: [PATCH 4/8] Fix linting issue --- cmd/src/snapshot_upload.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index a5f05a6077..e781efa8ea 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -24,6 +24,7 @@ const srcSnapshotDir = "./src-snapshot" // summaryFile on its own, as it gets handled a little differently const summaryFile = "summary.json" + var srcSnapshotSummaryPath = filepath.Join(srcSnapshotDir, summaryFile) // listOfValidFiles defines the valid snapshot filenames (with extensions) that can be uploaded @@ -56,7 +57,6 @@ type uploadFile struct { filterSQL bool // Whether to filter incompatible SQL statements during upload, true for database files, false for summary file } - func init() { usage := fmt.Sprintf(`'src snapshot upload' uploads instance snapshot contents generated by 'src snapshot databases' and 'src snapshot summary' to the designated bucket. From a7280e1881357a20f7f2795f7ea5759daef14a18 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Thu, 6 Nov 2025 20:40:43 -0500 Subject: [PATCH 5/8] Update code to match --file arg --- cmd/src/snapshot_upload.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index e781efa8ea..a5751f42a9 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -74,7 +74,7 @@ FILE flagSet := flag.NewFlagSet("upload", flag.ExitOnError) bucketName := flagSet.String("bucket", "", "destination Cloud Storage bucket name") credentialsPath := flagSet.String("credentials", "", "JSON credentials file for Google Cloud service account") - fileFilter := flagSet.String("file", strings.Join(listOfValidFiles, ","), "comma-delimited list of files to upload") + fileArg := flagSet.String("file", strings.Join(listOfValidFiles, ","), "comma-delimited list of files to upload") filterSQL := flagSet.Bool("filter-sql", true, "filter incompatible SQL statements from database dumps for import to Google Cloud SQL") // Register this command with the parent 'src snapshot' command. @@ -83,20 +83,20 @@ FILE // when their init() functions run, without requiring a central registry file. snapshotCommands = append(snapshotCommands, &command{ flagSet: flagSet, - handler: snapshotUploadHandler(flagSet, bucketName, credentialsPath, filterSQL, fileFilter), + handler: snapshotUploadHandler(flagSet, bucketName, credentialsPath, filterSQL, fileArg), usageFunc: func() { fmt.Fprint(flag.CommandLine.Output(), usage) }, }) } // Handler function to keep init() succinct -func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *string, filterSQL *bool, fileFilter *string) func([]string) error { +func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *string, filterSQL *bool, fileArg *string) func([]string) error { return func(args []string) error { if err := flagSet.Parse(args); err != nil { return err } // Validate and parse inputs into an uploadArgs-type object - uploadArgs, err := validateUploadInputs(*bucketName, *credentialsPath, *fileFilter, *filterSQL) + uploadArgs, err := validateUploadInputs(*bucketName, *credentialsPath, *fileArg, *filterSQL) if err != nil { return err } @@ -119,7 +119,7 @@ func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *s } // Validate user inputs, and convert them to an object of type uploadArgs -func validateUploadInputs(bucketName, credentialsPath, fileFilter string, filterSQL bool) (*uploadArgs, error) { +func validateUploadInputs(bucketName, credentialsPath, fileArg string, filterSQL bool) (*uploadArgs, error) { if bucketName == "" { return nil, errors.New("-bucket required") } @@ -127,7 +127,7 @@ func validateUploadInputs(bucketName, credentialsPath, fileFilter string, filter return nil, errors.New("-credentials required") } - filesToUpload, err := parseFileFilter(fileFilter) + filesToUpload, err := parseFileArg(fileArg) if err != nil { return nil, err } @@ -141,17 +141,17 @@ func validateUploadInputs(bucketName, credentialsPath, fileFilter string, filter } // Parse the --file arg values, and return a list of strings of the files to upload -func parseFileFilter(fileFilter string) ([]string, error) { +func parseFileArg(fileArg string) ([]string, error) { // Default: all files - if fileFilter == "" { + if fileArg == "" { return listOfValidFiles, nil } var filesToUpload []string // Parse comma-delimited list - for _, part := range strings.Split(fileFilter, ",") { + for _, part := range strings.Split(fileArg, ",") { // Trim whitespace filename := strings.TrimSpace(part) From 0d391d8720a31b1b9b81bb6879b01e428d85e1d1 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Sun, 9 Nov 2025 11:29:40 -0700 Subject: [PATCH 6/8] Fix printing helper text on invalid user input --- cmd/src/snapshot_upload.go | 47 +++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index a5751f42a9..20cd015910 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -16,6 +16,7 @@ import ( "github.com/sourcegraph/sourcegraph/lib/output" "google.golang.org/api/option" + "github.com/sourcegraph/src-cli/internal/cmderrors" "github.com/sourcegraph/src-cli/internal/pgdump" ) @@ -58,18 +59,39 @@ type uploadFile struct { } func init() { - usage := fmt.Sprintf(`'src snapshot upload' uploads instance snapshot contents generated by 'src snapshot databases' and 'src snapshot summary' to the designated bucket. + usage := fmt.Sprintf(` +'src snapshot upload' uploads the files generated by 'src snapshot databases' and 'src snapshot summary' to the specified GCS bucket, for self-hosted Sourcegraph customers migrating to Sourcegraph Cloud. USAGE - src snapshot upload -bucket=$MIGRATION_BUCKET_NAME -credentials=./migration_private_key.json [-file=$FILE] -BUCKET - In general, a Google Cloud Storage bucket and relevant credentials will be provided by Sourcegraph when using this functionality to share a snapshot with Sourcegraph. + src snapshot upload -bucket=$MIGRATION_BUCKET_NAME -credentials=$CREDENTIALS_FILE_PATH [-file] -FILE - Optional: Specify files to upload as a comma-delimited list (with file extensions). Valid values: %s. Default: All files. - Examples: -file=summary.json,pgsql.sql or -file=codeintel.sql -`, strings.Join(listOfValidFiles, ", ")) +EXAMPLES + + src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=path/to/migration_private_key.json + + src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=./migration_private_key.json -file=pgsql.sql + +ARGS + +-bucket + Name of the Google Cloud Storage bucket provided by Sourcegraph + Required + Type: string + +-credentials + File path to the credentials file provided by Sourcegraph + Required + Type: file path + +-file + Specify which files from the ./src-snapshot directory to upload, as a comma-delimited list of file names, with file-type extensions + Optional + Type: string + Valid values: %s + Default: All valid values + +`, strings.Join(listOfValidFiles, ",")) flagSet := flag.NewFlagSet("upload", flag.ExitOnError) bucketName := flagSet.String("bucket", "", "destination Cloud Storage bucket name") @@ -90,6 +112,7 @@ FILE // Handler function to keep init() succinct func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *string, filterSQL *bool, fileArg *string) func([]string) error { + return func(args []string) error { if err := flagSet.Parse(args); err != nil { return err @@ -120,11 +143,13 @@ func snapshotUploadHandler(flagSet *flag.FlagSet, bucketName, credentialsPath *s // Validate user inputs, and convert them to an object of type uploadArgs func validateUploadInputs(bucketName, credentialsPath, fileArg string, filterSQL bool) (*uploadArgs, error) { + if bucketName == "" { - return nil, errors.New("-bucket required") + return nil, cmderrors.Usage("-bucket required") } + if credentialsPath == "" { - return nil, errors.New("-credentials required") + return nil, cmderrors.Usage("-credentials required") } filesToUpload, err := parseFileArg(fileArg) @@ -158,7 +183,7 @@ func parseFileArg(fileArg string) ([]string, error) { // Validate against list of valid files if !slices.Contains(listOfValidFiles, filename) { - return nil, errors.Newf("invalid -file value %q. Valid values: %s", part, strings.Join(listOfValidFiles, ", ")) + return nil, cmderrors.Usagef("invalid -file value %q. Valid values: %s", part, strings.Join(listOfValidFiles, ", ")) } filesToUpload = append(filesToUpload, filename) From 05f12e5f7d8bcee353287e34f355af6b050f27b7 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Sun, 9 Nov 2025 12:30:44 -0700 Subject: [PATCH 7/8] Update helper text to match other src-cli modules --- cmd/src/snapshot.go | 18 ++++++++++++----- cmd/src/snapshot_upload.go | 41 +++++++++++++++++++------------------- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/cmd/src/snapshot.go b/cmd/src/snapshot.go index 0c604ca088..31048c1125 100644 --- a/cmd/src/snapshot.go +++ b/cmd/src/snapshot.go @@ -8,16 +8,24 @@ import ( var snapshotCommands commander func init() { - usage := `'src snapshot' manages snapshots of Sourcegraph instance data. All subcommands are currently EXPERIMENTAL. + usage := `'src snapshot' manages snapshots of Sourcegraph instance databases. All subcommands are currently EXPERIMENTAL. -USAGE - src [-v] snapshot +Usage: -COMMANDS + src snapshot + +The commands are: + + databases export databases from a Sourcegraph instance + restore restore databases from an export + upload upload exported databases and summary file when migrating to Sourcegraph Cloud summary export summary data about an instance for acceptance testing of a restored Sourcegraph instance test use exported summary data and instance health indicators to validate a restored and upgraded instance -` + +Use "src snapshot [command] -h" for more information about a command. + + ` flagSet := flag.NewFlagSet("snapshot", flag.ExitOnError) commands = append(commands, &command{ diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index 20cd015910..0316b76af3 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -59,39 +59,40 @@ type uploadFile struct { } func init() { - usage := fmt.Sprintf(` -'src snapshot upload' uploads the files generated by 'src snapshot databases' and 'src snapshot summary' to the specified GCS bucket, for self-hosted Sourcegraph customers migrating to Sourcegraph Cloud. + usage := fmt.Sprintf(`'src snapshot upload' uploads the files generated by 'src snapshot databases' and 'src snapshot summary' to the specified GCS bucket, for self-hosted Sourcegraph customers migrating to Sourcegraph Cloud. -USAGE +Usage: src snapshot upload -bucket=$MIGRATION_BUCKET_NAME -credentials=$CREDENTIALS_FILE_PATH [-file] -EXAMPLES +Examples: src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=path/to/migration_private_key.json src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=./migration_private_key.json -file=pgsql.sql -ARGS + src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=./migration_private_key.json -file="codeinsights.sql, codeintel.sql, pgsql.sql" --bucket - Name of the Google Cloud Storage bucket provided by Sourcegraph - Required - Type: string +Args: --credentials - File path to the credentials file provided by Sourcegraph - Required - Type: file path + -bucket + Name of the Google Cloud Storage bucket provided by Sourcegraph + Required + Type: string --file - Specify which files from the ./src-snapshot directory to upload, as a comma-delimited list of file names, with file-type extensions - Optional - Type: string - Valid values: %s - Default: All valid values + -credentials + File path to the credentials file provided by Sourcegraph + Required + Type: file path, as string -`, strings.Join(listOfValidFiles, ",")) + -file + Specify which files from the ./src-snapshot directory to upload + Optional + Type: comma-delimited list of file names, with file-type extensions, as a string + Valid values: %s + Default: All valid values + +`, strings.Join(listOfValidFiles, ", ")) flagSet := flag.NewFlagSet("upload", flag.ExitOnError) bucketName := flagSet.String("bucket", "", "destination Cloud Storage bucket name") From 897e757e9d77270c2735ac2661d0eb813aa2767a Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Sun, 9 Nov 2025 15:58:02 -0700 Subject: [PATCH 8/8] Adjust usage text --- cmd/src/snapshot.go | 2 +- cmd/src/snapshot_upload.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/src/snapshot.go b/cmd/src/snapshot.go index 31048c1125..837ed3c21a 100644 --- a/cmd/src/snapshot.go +++ b/cmd/src/snapshot.go @@ -25,7 +25,7 @@ The commands are: Use "src snapshot [command] -h" for more information about a command. - ` +` flagSet := flag.NewFlagSet("snapshot", flag.ExitOnError) commands = append(commands, &command{ diff --git a/cmd/src/snapshot_upload.go b/cmd/src/snapshot_upload.go index 0316b76af3..86c141053b 100644 --- a/cmd/src/snapshot_upload.go +++ b/cmd/src/snapshot_upload.go @@ -67,11 +67,11 @@ Usage: Examples: - src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=path/to/migration_private_key.json + src snapshot upload -bucket=example-bucket-name -credentials=path/to/migration_private_key.json - src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=./migration_private_key.json -file=pgsql.sql + src snapshot upload -bucket=example-bucket-name -credentials=./migration_private_key.json -file=pgsql.sql - src snapshot upload -bucket=sourcegraph-cloud-data-migration-example-bucket -credentials=./migration_private_key.json -file="codeinsights.sql, codeintel.sql, pgsql.sql" + src snapshot upload -bucket=example-bucket-name -credentials=./migration_private_key.json -file="codeinsights.sql, codeintel.sql, pgsql.sql" Args: @@ -95,10 +95,10 @@ Args: `, strings.Join(listOfValidFiles, ", ")) flagSet := flag.NewFlagSet("upload", flag.ExitOnError) - bucketName := flagSet.String("bucket", "", "destination Cloud Storage bucket name") - credentialsPath := flagSet.String("credentials", "", "JSON credentials file for Google Cloud service account") - fileArg := flagSet.String("file", strings.Join(listOfValidFiles, ","), "comma-delimited list of files to upload") - filterSQL := flagSet.Bool("filter-sql", true, "filter incompatible SQL statements from database dumps for import to Google Cloud SQL") + bucketName := flagSet.String("bucket", "", "Name of the Google Cloud Storage bucket provided by Sourcegraph") + credentialsPath := flagSet.String("credentials", "", "File path to the credentials file provided by Sourcegraph") + fileArg := flagSet.String("file", strings.Join(listOfValidFiles, ","), "Specify which files from the ./src-snapshot directory to upload") + filterSQL := flagSet.Bool("filter-sql", true, "Filter incompatible SQL statements from database snapshots which break the import into Google Cloud SQL") // Register this command with the parent 'src snapshot' command. // The parent snapshot.go command runs all registered subcommands via snapshotCommands.run().