From 94be2c3594b3ed379f808f4e178eeede0a4dbc01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Kutryj?= Date: Thu, 21 Aug 2025 11:07:54 +0200 Subject: [PATCH 1/2] feat: Log stats of artifact push command --- cmd/pull.go | 14 --- cmd/push.go | 11 +- cmd/utils.go | 17 +++ pkg/storage/pull.go | 6 +- pkg/storage/push.go | 41 +++++-- pkg/storage/push_test.go | 248 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 305 insertions(+), 32 deletions(-) create mode 100644 cmd/utils.go create mode 100644 pkg/storage/push_test.go diff --git a/cmd/pull.go b/cmd/pull.go index 37755ad..39c1c5b 100644 --- a/cmd/pull.go +++ b/cmd/pull.go @@ -1,7 +1,6 @@ package cmd import ( - "fmt" errutil "github.com/semaphoreci/artifact/pkg/errors" "github.com/semaphoreci/artifact/pkg/files" "github.com/semaphoreci/artifact/pkg/hub" @@ -141,19 +140,6 @@ func NewPullProjectCmd() *cobra.Command { return cmd } -// formatBytes converts bytes to human readable format -func formatBytes(bytes int64) string { - const unit = 1024 - if bytes < unit { - return fmt.Sprintf("%d B", bytes) - } - div, exp := int64(unit), 0 - for n := bytes / unit; n >= unit; n /= unit { - div *= unit - exp++ - } - return fmt.Sprintf("%.1f %cB", float64(bytes)/float64(div), "KMGTPE"[exp]) -} func init() { rootCmd.AddCommand(pullCmd) diff --git a/cmd/push.go b/cmd/push.go index df6a817..3d2377e 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -35,7 +35,7 @@ var pushCmd = &cobra.Command{ while the rest of the semaphore process, or after it.`, } -func runPushForCategory(cmd *cobra.Command, args []string, resolver *files.PathResolver) (*files.ResolvedPath, error) { +func runPushForCategory(cmd *cobra.Command, args []string, resolver *files.PathResolver) (*files.ResolvedPath, *storage.PushStats, error) { hubClient, err := hub.NewClient() errutil.Check(err) @@ -83,7 +83,7 @@ func NewPushJobCmd() *cobra.Command { resolver, err := files.NewPathResolver(files.ResourceTypeJob, jobId) errutil.Check(err) - paths, err := runPushForCategory(cmd, args, resolver) + paths, stats, err := runPushForCategory(cmd, args, resolver) if err != nil { log.Errorf("Error pushing artifact: %v\n", err) errutil.Exit(1) @@ -93,6 +93,7 @@ func NewPushJobCmd() *cobra.Command { log.Info("Successfully pushed artifact for current job.\n") log.Infof("* Local source: %s.\n", paths.Source) log.Infof("* Remote destination: %s.\n", paths.Destination) + log.Infof("Pushed %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) }, } @@ -118,7 +119,7 @@ func NewPushWorkflowCmd() *cobra.Command { resolver, err := files.NewPathResolver(files.ResourceTypeWorkflow, workflowId) errutil.Check(err) - paths, err := runPushForCategory(cmd, args, resolver) + paths, stats, err := runPushForCategory(cmd, args, resolver) if err != nil { log.Errorf("Error pushing artifact: %v\n", err) errutil.Exit(1) @@ -128,6 +129,7 @@ func NewPushWorkflowCmd() *cobra.Command { log.Info("Successfully pushed artifact for current workflow.\n") log.Infof("* Local source: %s.\n", paths.Source) log.Infof("* Remote destination: %s.\n", paths.Destination) + log.Infof("Pushed %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) }, } @@ -153,7 +155,7 @@ func NewPushProjectCmd() *cobra.Command { resolver, err := files.NewPathResolver(files.ResourceTypeProject, projectId) errutil.Check(err) - paths, err := runPushForCategory(cmd, args, resolver) + paths, stats, err := runPushForCategory(cmd, args, resolver) if err != nil { log.Errorf("Error pushing artifact: %v\n", err) errutil.Exit(1) @@ -163,6 +165,7 @@ func NewPushProjectCmd() *cobra.Command { log.Info("Successfully pushed artifact for current project.\n") log.Infof("* Local source: %s.\n", paths.Source) log.Infof("* Remote destination: %s.\n", paths.Destination) + log.Infof("Pushed %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) }, } diff --git a/cmd/utils.go b/cmd/utils.go new file mode 100644 index 0000000..5a736aa --- /dev/null +++ b/cmd/utils.go @@ -0,0 +1,17 @@ +package cmd + +import "fmt" + +// formatBytes converts bytes to human readable format +func formatBytes(bytes int64) string { + const unit = 1024 + if bytes < unit { + return fmt.Sprintf("%d B", bytes) + } + div, exp := int64(unit), 0 + for n := bytes / unit; n >= unit; n /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f %cB", float64(bytes)/float64(div), "KMGTPE"[exp]) +} \ No newline at end of file diff --git a/pkg/storage/pull.go b/pkg/storage/pull.go index 2d040a7..a46330e 100644 --- a/pkg/storage/pull.go +++ b/pkg/storage/pull.go @@ -43,7 +43,7 @@ func Pull(hubClient *hub.Client, resolver *files.PathResolver, options PullOptio return nil, nil, err } - stats, err := doPull(options.Force, artifacts, response.Urls) + stats, err := doPull(artifacts) if err != nil { return nil, nil, err } @@ -78,7 +78,7 @@ func buildArtifacts(signedURLs []*api.SignedURL, paths *files.ResolvedPath, forc return artifacts, nil } -func doPull(force bool, artifacts []*api.Artifact, signedURLs []*api.SignedURL) (*PullStats, error) { +func doPull(artifacts []*api.Artifact) (*PullStats, error) { client := newHTTPClient() stats := &PullStats{} @@ -87,7 +87,7 @@ func doPull(force bool, artifacts []*api.Artifact, signedURLs []*api.SignedURL) if err := signedURL.Follow(client, artifact); err != nil { return nil, err } - + // Get file size after successful download if fileInfo, err := os.Stat(artifact.LocalPath); err == nil { stats.FileCount++ diff --git a/pkg/storage/push.go b/pkg/storage/push.go index 258d1f0..0990d2c 100644 --- a/pkg/storage/push.go +++ b/pkg/storage/push.go @@ -18,6 +18,11 @@ type PushOptions struct { Force bool } +type PushStats struct { + FileCount int + TotalSize int64 +} + func (o *PushOptions) RequestType() hub.GenerateSignedURLsRequestType { if o.Force { return hub.GenerateSignedURLsRequestPUSHFORCE @@ -26,10 +31,10 @@ func (o *PushOptions) RequestType() hub.GenerateSignedURLsRequestType { return hub.GenerateSignedURLsRequestPUSH } -func Push(hubClient *hub.Client, resolver *files.PathResolver, options PushOptions) (*files.ResolvedPath, error) { +func Push(hubClient *hub.Client, resolver *files.PathResolver, options PushOptions) (*files.ResolvedPath, *PushStats, error) { paths, err := resolver.Resolve(files.OperationPush, options.SourcePath, options.DestinationOverride) if err != nil { - return nil, err + return nil, nil, err } log.Debug("Pushing...\n") @@ -39,25 +44,25 @@ func Push(hubClient *hub.Client, resolver *files.PathResolver, options PushOptio artifacts, err := LocateArtifacts(paths) if err != nil { - return nil, err + return nil, nil, err } response, err := hubClient.GenerateSignedURLs(api.RemotePaths(artifacts), options.RequestType()) if err != nil { - return nil, err + return nil, nil, err } err = attachURLs(artifacts, response.Urls, options.Force) if err != nil { - return nil, err + return nil, nil, err } - err = doPush(options.Force, artifacts, response.Urls) + stats, err := doPush(artifacts) if err != nil { - return nil, err + return nil, nil, err } - return paths, nil + return paths, stats, nil } func LocateArtifacts(paths *files.ResolvedPath) ([]*api.Artifact, error) { @@ -131,16 +136,30 @@ func attachURLs(items []*api.Artifact, signedURLs []*api.SignedURL, force bool) return nil } -func doPush(force bool, artifacts []*api.Artifact, signedURLs []*api.SignedURL) error { +func doPush(artifacts []*api.Artifact) (*PushStats, error) { client := newHTTPClient() + stats := &PushStats{} for _, artifact := range artifacts { + fileInfo, err := os.Stat(artifact.LocalPath) + if err != nil { + return nil, fmt.Errorf("failed to stat '%s': %v", artifact.LocalPath, err) + } + for _, signedURL := range artifact.URLs { if err := signedURL.Follow(client, artifact); err != nil { - return err + return nil, err + } + } + + for _, url := range artifact.URLs { + if url.Method == "PUT" { + stats.FileCount++ + stats.TotalSize += fileInfo.Size() + break } } } - return nil + return stats, nil } diff --git a/pkg/storage/push_test.go b/pkg/storage/push_test.go new file mode 100644 index 0000000..733e750 --- /dev/null +++ b/pkg/storage/push_test.go @@ -0,0 +1,248 @@ +package storage + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/semaphoreci/artifact/pkg/api" + "github.com/semaphoreci/artifact/pkg/files" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test__doPush_Stats_SingleFile(t *testing.T) { + tempDir, err := ioutil.TempDir("", "push_test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + testFile := filepath.Join(tempDir, "test.txt") + content := []byte("hello world") + err = ioutil.WriteFile(testFile, content, 0644) + require.NoError(t, err) + + artifact := &api.Artifact{ + RemotePath: "test.txt", + LocalPath: testFile, + URLs: []*api.SignedURL{ + {Method: "PUT", URL: "https://example.com/upload"}, + }, + } + + stats := &PushStats{} + + fileInfo, err := os.Stat(artifact.LocalPath) + require.NoError(t, err) + + for _, url := range artifact.URLs { + if url.Method == "PUT" { + stats.FileCount++ + stats.TotalSize += fileInfo.Size() + break + } + } + + assert.Equal(t, 1, stats.FileCount) + assert.Equal(t, int64(11), stats.TotalSize) +} + +func Test__doPush_Stats_MultipleFiles(t *testing.T) { + tempDir, err := ioutil.TempDir("", "push_multi_test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + testFiles := []struct { + name string + content string + size int64 + }{ + {"file1.txt", "hello world", 11}, + {"file2.txt", "test content here", 17}, + {"file3.txt", "a", 1}, + {"file4.txt", "longer content for testing purposes", 35}, + } + + artifacts := []*api.Artifact{} + for _, tf := range testFiles { + localPath := filepath.Join(tempDir, tf.name) + err := ioutil.WriteFile(localPath, []byte(tf.content), 0644) + require.NoError(t, err) + + artifacts = append(artifacts, &api.Artifact{ + RemotePath: tf.name, + LocalPath: localPath, + URLs: []*api.SignedURL{ + {Method: "PUT", URL: "https://example.com/upload/" + tf.name}, + }, + }) + } + + stats := &PushStats{} + + for _, artifact := range artifacts { + fileInfo, err := os.Stat(artifact.LocalPath) + require.NoError(t, err) + + for _, url := range artifact.URLs { + if url.Method == "PUT" { + stats.FileCount++ + stats.TotalSize += fileInfo.Size() + break + } + } + } + + assert.Equal(t, 4, stats.FileCount) + assert.Equal(t, int64(64), stats.TotalSize) +} + +func Test__PushStats_EmptyList(t *testing.T) { + stats := &PushStats{} + artifacts := []*api.Artifact{} + + for _, artifact := range artifacts { + fileInfo, err := os.Stat(artifact.LocalPath) + if err == nil { + for _, url := range artifact.URLs { + if url.Method == "PUT" { + stats.FileCount++ + stats.TotalSize += fileInfo.Size() + break + } + } + } + } + + assert.Equal(t, 0, stats.FileCount) + assert.Equal(t, int64(0), stats.TotalSize) +} + +func Test__PushStats_LargeFile(t *testing.T) { + tempDir, err := ioutil.TempDir("", "push_large_test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + largeContent := make([]byte, 1024*1024) + for i := range largeContent { + largeContent[i] = byte(i % 256) + } + + localPath := filepath.Join(tempDir, "large_file.bin") + err = ioutil.WriteFile(localPath, largeContent, 0644) + require.NoError(t, err) + + artifact := &api.Artifact{ + RemotePath: "large_file.bin", + LocalPath: localPath, + URLs: []*api.SignedURL{ + {Method: "PUT", URL: "https://example.com/upload/large_file.bin"}, + }, + } + + stats := &PushStats{} + + fileInfo, err := os.Stat(artifact.LocalPath) + require.NoError(t, err) + + for _, url := range artifact.URLs { + if url.Method == "PUT" { + stats.FileCount++ + stats.TotalSize += fileInfo.Size() + break + } + } + + assert.Equal(t, 1, stats.FileCount) + assert.Equal(t, int64(1024*1024), stats.TotalSize) +} + +func Test__PushStats_NonForceWithHEAD(t *testing.T) { + tempDir, err := ioutil.TempDir("", "push_head_test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + testFile := filepath.Join(tempDir, "test.txt") + content := []byte("test content") + err = ioutil.WriteFile(testFile, content, 0644) + require.NoError(t, err) + + artifact := &api.Artifact{ + RemotePath: "test.txt", + LocalPath: testFile, + URLs: []*api.SignedURL{ + {Method: "HEAD", URL: "https://example.com/check"}, + {Method: "PUT", URL: "https://example.com/upload"}, + }, + } + + stats := &PushStats{} + + fileInfo, err := os.Stat(artifact.LocalPath) + require.NoError(t, err) + + for _, url := range artifact.URLs { + if url.Method == "PUT" { + stats.FileCount++ + stats.TotalSize += fileInfo.Size() + break + } + } + + assert.Equal(t, 1, stats.FileCount) + assert.Equal(t, int64(12), stats.TotalSize) +} + +func Test__LocateArtifacts_SingleFile(t *testing.T) { + tempDir, err := ioutil.TempDir("", "locate_test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + testFile := filepath.Join(tempDir, "single.txt") + err = ioutil.WriteFile(testFile, []byte("content"), 0644) + require.NoError(t, err) + + paths := &files.ResolvedPath{ + Source: testFile, + Destination: "remote/single.txt", + } + + artifacts, err := LocateArtifacts(paths) + require.NoError(t, err) + + assert.Equal(t, 1, len(artifacts)) + assert.Equal(t, testFile, artifacts[0].LocalPath) + assert.Equal(t, "remote/single.txt", artifacts[0].RemotePath) +} + +func Test__LocateArtifacts_Directory(t *testing.T) { + tempDir, err := ioutil.TempDir("", "locate_dir_test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + subDir := filepath.Join(tempDir, "subdir") + err = os.Mkdir(subDir, 0755) + require.NoError(t, err) + + file1 := filepath.Join(tempDir, "file1.txt") + file2 := filepath.Join(subDir, "file2.txt") + + err = ioutil.WriteFile(file1, []byte("content1"), 0644) + require.NoError(t, err) + err = ioutil.WriteFile(file2, []byte("content2"), 0644) + require.NoError(t, err) + + paths := &files.ResolvedPath{ + Source: tempDir, + Destination: "remote/dir", + } + + artifacts, err := LocateArtifacts(paths) + require.NoError(t, err) + + assert.Equal(t, 2, len(artifacts)) + + localPaths := []string{artifacts[0].LocalPath, artifacts[1].LocalPath} + assert.Contains(t, localPaths, file1) + assert.Contains(t, localPaths, file2) +} From e0d10998cdd3b121d8df6d2471fb750a06a1089d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Kutryj?= Date: Thu, 21 Aug 2025 11:32:21 +0200 Subject: [PATCH 2/2] toil: pluralize number of files in stats --- cmd/pull.go | 6 +++--- cmd/push.go | 6 +++--- cmd/utils.go | 8 ++++++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cmd/pull.go b/cmd/pull.go index 39c1c5b..d87ac0c 100644 --- a/cmd/pull.go +++ b/cmd/pull.go @@ -60,7 +60,7 @@ func NewPullJobCmd() *cobra.Command { log.Info("Successfully pulled artifact for current job.\n") log.Infof("* Remote source: '%s'.\n", paths.Source) log.Infof("* Local destination: '%s'.\n", paths.Destination) - log.Infof("Pulled %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) + log.Infof("Pulled %d %s. Total of %s\n", stats.FileCount, pluralize(stats.FileCount, "file", "files"), formatBytes(stats.TotalSize)) }, } @@ -95,7 +95,7 @@ func NewPullWorkflowCmd() *cobra.Command { log.Info("Successfully pulled artifact for current workflow.\n") log.Infof("* Remote source: '%s'.\n", paths.Source) log.Infof("* Local destination: '%s'.\n", paths.Destination) - log.Infof("Pulled %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) + log.Infof("Pulled %d %s. Total of %s\n", stats.FileCount, pluralize(stats.FileCount, "file", "files"), formatBytes(stats.TotalSize)) }, } @@ -130,7 +130,7 @@ func NewPullProjectCmd() *cobra.Command { log.Info("Successfully pulled artifact for current project.\n") log.Infof("* Remote source: '%s'.\n", paths.Source) log.Infof("* Local destination: '%s'.\n", paths.Destination) - log.Infof("Pulled %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) + log.Infof("Pulled %d %s. Total of %s\n", stats.FileCount, pluralize(stats.FileCount, "file", "files"), formatBytes(stats.TotalSize)) }, } diff --git a/cmd/push.go b/cmd/push.go index 3d2377e..7263dec 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -93,7 +93,7 @@ func NewPushJobCmd() *cobra.Command { log.Info("Successfully pushed artifact for current job.\n") log.Infof("* Local source: %s.\n", paths.Source) log.Infof("* Remote destination: %s.\n", paths.Destination) - log.Infof("Pushed %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) + log.Infof("Pushed %d %s. Total of %s\n", stats.FileCount, pluralize(stats.FileCount, "file", "files"), formatBytes(stats.TotalSize)) }, } @@ -129,7 +129,7 @@ func NewPushWorkflowCmd() *cobra.Command { log.Info("Successfully pushed artifact for current workflow.\n") log.Infof("* Local source: %s.\n", paths.Source) log.Infof("* Remote destination: %s.\n", paths.Destination) - log.Infof("Pushed %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) + log.Infof("Pushed %d %s. Total of %s\n", stats.FileCount, pluralize(stats.FileCount, "file", "files"), formatBytes(stats.TotalSize)) }, } @@ -165,7 +165,7 @@ func NewPushProjectCmd() *cobra.Command { log.Info("Successfully pushed artifact for current project.\n") log.Infof("* Local source: %s.\n", paths.Source) log.Infof("* Remote destination: %s.\n", paths.Destination) - log.Infof("Pushed %d files. Total of %s\n", stats.FileCount, formatBytes(stats.TotalSize)) + log.Infof("Pushed %d %s. Total of %s\n", stats.FileCount, pluralize(stats.FileCount, "file", "files"), formatBytes(stats.TotalSize)) }, } diff --git a/cmd/utils.go b/cmd/utils.go index 5a736aa..e9c0067 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -14,4 +14,12 @@ func formatBytes(bytes int64) string { exp++ } return fmt.Sprintf("%.1f %cB", float64(bytes)/float64(div), "KMGTPE"[exp]) +} + +// pluralize returns singular or plural form based on count +func pluralize(count int, singular, plural string) string { + if count == 1 { + return singular + } + return plural } \ No newline at end of file