From 06a22c5b74e8caf24475dc8960a3ee8d4c0d4813 Mon Sep 17 00:00:00 2001
From: guyhardonag <guy.hardonag@treeverse.io>
Date: Thu, 11 Jul 2024 17:46:29 +0300
Subject: [PATCH] Code review changes

---
 cmd/lakectl/cmd/fs_upload.go | 21 ++++++++++++---------
 esti/lakectl_test.go         |  7 ++++---
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/cmd/lakectl/cmd/fs_upload.go b/cmd/lakectl/cmd/fs_upload.go
index 9ca644cdddc..6d4c95cce18 100644
--- a/cmd/lakectl/cmd/fs_upload.go
+++ b/cmd/lakectl/cmd/fs_upload.go
@@ -32,16 +32,8 @@ var fsUploadCmd = &cobra.Command{
 
 		ctx, stop := signal.NotifyContext(ctx, os.Interrupt, os.Kill)
 		defer stop()
-		ignoreRecessive := true // Ignore recursive if source is the standard input or a file (not a directory)
-		if source != StdinFileName {
-			stat, err := os.Stat(source)
-			if err != nil {
-				Die("failed to stat source", 1)
-			}
-			ignoreRecessive = !stat.IsDir()
-		}
 
-		if !recursive || ignoreRecessive {
+		if !recursive || isFileOrStdin(source) {
 			if strings.HasSuffix(remotePath, uri.PathSeparator) {
 				Die("target path is not a valid URI", 1)
 			}
@@ -88,6 +80,17 @@ var fsUploadCmd = &cobra.Command{
 	},
 }
 
+func isFileOrStdin(source string) bool {
+	if source == StdinFileName {
+		return true
+	}
+	stat, err := os.Stat(source)
+	if err != nil {
+		Die("failed to stat source", 1)
+	}
+	return !stat.IsDir()
+}
+
 func upload(ctx context.Context, client apigen.ClientWithResponsesInterface, sourcePathname string, destURI *uri.URI, contentType string, syncFlags local.SyncFlags) (*apigen.ObjectStats, error) {
 	fp := Must(OpenByPath(sourcePathname))
 	defer func() {
diff --git a/esti/lakectl_test.go b/esti/lakectl_test.go
index ac2eae435ae..ed6946fba77 100644
--- a/esti/lakectl_test.go
+++ b/esti/lakectl_test.go
@@ -112,13 +112,14 @@ func TestLakectlPreSignUpload(t *testing.T) {
 	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" log lakefs://"+repoName+"/"+mainBranch, false, "lakectl_log_initial", vars)
 
 	filePath := "ro_1k.1"
-	vars["FILE_PATH"] = filePath
 	t.Run("upload from file", func(t *testing.T) {
+		vars["FILE_PATH"] = filePath
 		RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k lakefs://"+repoName+"/"+mainBranch+"/"+filePath+" --pre-sign", false, "lakectl_fs_upload", vars)
 	})
 	t.Run("upload from stdin", func(t *testing.T) {
-		filePath = "sample"
-		RunCmdAndVerifySuccess(t, "echo sample | "+Lakectl()+" -c ~/.lakectl.yaml fs upload -s - lakefs://"+repoName+"/"+mainBranch+"/"+filePath+" --pre-sign", false, "sample", vars)
+		filePath = "ro_1k.2"
+		vars["FILE_PATH"] = filePath
+		RunCmdAndVerifySuccess(t, "echo sample | "+Lakectl()+" fs upload -s - lakefs://"+repoName+"/"+mainBranch+"/"+filePath+" --pre-sign", false, "sample", vars)
 	})
 }