Repository: bacalhau-project/bacalhau (language: python)

Issue:
Move `bacalhau get` to `bacalhau job get`
today we have `bacalhau get` to fetch results. We v2 CLIs we started to introduce the resource type the command relates to, such as `bacalhau job describe` instead of `bacalhau describe`, and `bacalhau job list` instead of `bacalhau list`

This issue is about introducing `bacalhau job get` that should act similarly to `bacalhau get`

If `bacalhau get` is used print the error message:
```
This command has moved! Please use `job get` to download job results.
```


Discussion / hints:

Fix patch:
diff --git a/cmd/cli/get/get.go b/cmd/cli/get/get.go
index 8589610b42..8e9b3cfaa1 100644
--- a/cmd/cli/get/get.go
+++ b/cmd/cli/get/get.go
@@ -2,29 +2,13 @@ package get
 
 import (
 	"github.com/spf13/cobra"
-	"k8s.io/kubectl/pkg/util/i18n"
-
-	"github.com/bacalhau-project/bacalhau/cmd/cli/job"
-	"github.com/bacalhau-project/bacalhau/cmd/util/flags/cliflags"
-	"github.com/bacalhau-project/bacalhau/pkg/util/templates"
-)
-
-var (
-	getExample = templates.Examples(i18n.T(`
-		# Get the results of a job.
-		bacalhau get 51225160-807e-48b8-88c9-28311c7899e1
-
-		# Get the results of a job, with a short ID.
-		bacalhau get ebd9bf2f
-`))
 )
 
-type GetOptions struct {
-	DownloadSettings *cliflags.DownloaderSettings
-}
-
 func NewCmd() *cobra.Command {
-	cmd := job.NewGetCmd()
-	cmd.Example = getExample
-	return cmd
+	return &cobra.Command{
+		Use:        "get [id]",
+		Short:      "DEPRECATED: Get the results of a job",
+		Deprecated: "This command has moved! Please use `job get` to download results of a job.",
+		RunE:       func(cmd *cobra.Command, cmdArgs []string) error { return nil },
+	}
 }
diff --git a/cmd/cli/job/get.go b/cmd/cli/job/get.go
index 05944feb72..cf1f7a8c47 100644
--- a/cmd/cli/job/get.go
+++ b/cmd/cli/job/get.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"k8s.io/kubectl/pkg/util/i18n"
@@ -26,7 +25,7 @@ var (
 	//nolint:lll // Documentation
 	getExample = templates.Examples(i18n.T(`
 		# Get the results of a job.
-		bacalhau job get 51225160-807e-48b8-88c9-28311c7899e1
+		bacalhau job get j-51225160-807e-48b8-88c9-28311c7899e1
 
 		# Get the results of a job, with a short ID.
 		bacalhau job get ebd9bf2f
@@ -109,7 +108,7 @@ func get(cmd *cobra.Command, cmdArgs []string, api client.API, cfg types.Bacalha
 		jobID,
 		OG.DownloadSettings,
 	); err != nil {
-		return errors.Wrap(err, "error downloading job")
+		return fmt.Errorf("downloading job: %w", err)
 	}
 
 	return nil


Test patch:
diff --git a/cmd/cli/job/get_test.go b/cmd/cli/job/get_test.go
index 324dcaeaea..9c0b2a3627 100644
--- a/cmd/cli/job/get_test.go
+++ b/cmd/cli/job/get_test.go
@@ -11,16 +11,16 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
-	"github.com/bacalhau-project/bacalhau/pkg/downloader"
-	testutils "github.com/bacalhau-project/bacalhau/pkg/test/utils"
-
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
-	cmdtesting "github.com/bacalhau-project/bacalhau/cmd/testing"
 	"github.com/bacalhau-project/bacalhau/cmd/util"
-	"github.com/bacalhau-project/bacalhau/pkg/docker"
+	"github.com/bacalhau-project/bacalhau/pkg/downloader"
 	"github.com/bacalhau-project/bacalhau/pkg/system"
+	testutils "github.com/bacalhau-project/bacalhau/pkg/test/utils"
+
+	cmdtesting "github.com/bacalhau-project/bacalhau/cmd/testing"
+	"github.com/bacalhau-project/bacalhau/pkg/docker"
 )
 
 // In order for 'go test' to run this suite, we need to create
@@ -42,148 +42,6 @@ func (s *GetSuite) SetupTest() {
 	s.BaseSuite.SetupTest()
 }
 
-func testResultsFolderStructure(t *testing.T, baseFolder string, expectedFiles []string) {
-	var files []string
-	err := filepath.Walk(baseFolder, func(path string, _ os.FileInfo, _ error) error {
-		usePath := strings.Replace(path, baseFolder, "", 1)
-		if usePath != "" {
-			files = append(files, usePath)
-		}
-		return nil
-	})
-	require.NoError(t, err, "Error walking results directory")
-
-	var expected []string
-	if expectedFiles != nil {
-		expected = expectedFiles
-	} else {
-		// Default folder structure if nothing was provided
-		expected = []string{
-			"/data",
-			"/data/apples",
-			"/data/apples/file.txt",
-			"/data/file.txt",
-			"/exitCode",
-			"/outputs",
-			"/" + downloader.DownloadFilenameStderr,
-			"/" + downloader.DownloadFilenameStdout,
-		}
-	}
-
-	require.Equal(t, strings.Join(expected, "\n"), strings.Join(files, "\n"), "The discovered results output structure was not correct")
-}
-
-func testDownloadOutput(t *testing.T, cmdOutput, jobID, outputDir string) {
-	require.True(t, strings.Contains(
-		cmdOutput,
-		fmt.Sprintf("Results for job '%s'", jobID),
-	), "Job ID not found in output")
-	require.True(t, strings.Contains(
-		cmdOutput,
-		outputDir,
-	), "Download location not found in output")
-
-}
-
-func setupTempWorkingDir(t *testing.T) (string, func()) {
-	// switch wd to a temp dir so we are not writing folders to the current directory
-	// (the point of this test is to see what happens when we DON'T pass --output-dir)
-	tempDir := t.TempDir()
-	originalWd, err := os.Getwd()
-	require.NoError(t, err)
-	err = os.Chdir(tempDir)
-	require.NoError(t, err)
-	// Below looks redundant but it is necessary on Darwin which does some
-	// freaky symlinking of root dirs: https://unix.stackexchange.com/a/63565
-	newTempDir, err := os.Getwd()
-	require.NoError(t, err)
-	return newTempDir, func() {
-		assert.NoError(t, os.Chdir(originalWd))
-	}
-}
-
-func (s *GetSuite) getDockerRunArgs(extraArgs []string) []string {
-	args := []string{
-		"docker", "run",
-		"--publisher", "local",
-		"-o", "data:/data",
-		"--wait",
-	}
-	args = append(args, extraArgs...)
-	args = append(args,
-		"ubuntu:kinetic",
-		"--",
-		"bash", "-c",
-		"echo hello > /data/file.txt && echo hello && mkdir /data/apples && echo oranges > /data/apples/file.txt",
-	)
-	return args
-}
-
-// this tests that when we do docker run with no --output-dir
-// it makes it's own folder to put the results in and does not splat results
-// all over the current directory
-func (s *GetSuite) TestDockerRunWriteToJobFolderAutoDownload() {
-	s.T().Skip("--download not supported in v2")
-	tempDir, cleanup := setupTempWorkingDir(s.T())
-	defer cleanup()
-
-	args := s.getDockerRunArgs([]string{
-		"--wait",
-		"--download",
-	})
-	_, runOutput, err := s.ExecuteTestCobraCommand(args...)
-	require.NoError(s.T(), err, "Error submitting job")
-
-	jobID := system.FindJobIDInTestOutputLegacy(runOutput)
-	outputFolder := filepath.Join(tempDir, util.GetDefaultJobFolder(jobID))
-	testDownloadOutput(s.T(), runOutput, jobID, tempDir)
-	testResultsFolderStructure(s.T(), outputFolder, nil)
-
-}
-
-// this tests that when we do docker run with an --output-dir
-// the results layout adheres to the expected folder layout
-func (s *GetSuite) TestDockerRunWriteToJobFolderNamedDownload() {
-	s.T().Skip("--download not supported in v2")
-	tempDir, err := os.MkdirTemp("", "docker-run-download-test")
-	require.NoError(s.T(), err)
-
-	args := s.getDockerRunArgs([]string{
-		"--wait",
-		"--download",
-		"--output-dir", tempDir,
-	})
-	_, runOutput, err := s.ExecuteTestCobraCommand(args...)
-	require.NoError(s.T(), err, "Error submitting job")
-	jobID := system.FindJobIDInTestOutputLegacy(runOutput)
-	testDownloadOutput(s.T(), runOutput, jobID, tempDir)
-	testResultsFolderStructure(s.T(), tempDir, nil)
-}
-
-// this tests that when we do get with no --output-dir
-// it makes it's own folder to put the results in and does not splat results
-// all over the current directory
-func (s *GetSuite) TestGetWriteToJobFolderAutoDownload() {
-	s.T().Skip("--download not supported in v2")
-	tempDir, cleanup := setupTempWorkingDir(s.T())
-	defer cleanup()
-
-	args := s.getDockerRunArgs([]string{
-		"--wait",
-		"--publisher", "local",
-	})
-	_, out, err := s.ExecuteTestCobraCommand(args...)
-	require.NoError(s.T(), err, "Error submitting job")
-	jobID := system.FindJobIDInTestOutputLegacy(out)
-	_, getOutput, err := s.ExecuteTestCobraCommand("job", "get",
-		jobID,
-	)
-	require.NoError(s.T(), err, "Error getting results")
-
-	testDownloadOutput(s.T(), getOutput, jobID, filepath.Join(tempDir, util.GetDefaultJobFolder(jobID)))
-	testResultsFolderStructure(s.T(), filepath.Join(tempDir, util.GetDefaultJobFolder(jobID)), nil)
-}
-
 func (s *GetSuite) TestGetSingleFileFromOutputBadChoice() {
 	testutils.MustHaveIPFS(s.T(), s.Config.Node.IPFS.Connect)
 	args := s.getDockerRunArgs([]string{
@@ -192,7 +50,7 @@ func (s *GetSuite) TestGetSingleFileFromOutputBadChoice() {
 	})
 	_, out, err := s.ExecuteTestCobraCommand(args...)
 	require.NoError(s.T(), err, "Error submitting job")
-	jobID := system.FindJobIDInTestOutputLegacy(out)
+	jobID := system.FindJobIDInTestOutput(out)
 
 	_, getoutput, err := s.ExecuteTestCobraCommand("job", "get",
 		"--ipfs-connect", s.Config.Node.IPFS.Connect,
@@ -214,7 +72,8 @@ func (s *GetSuite) TestGetSingleFileFromOutput() {
 	})
 	_, out, err := s.ExecuteTestCobraCommand(args...)
 	require.NoError(s.T(), err, "Error submitting job")
-	jobID := system.FindJobIDInTestOutputLegacy(out)
+	jobID := system.FindJobIDInTestOutput(out)
+	hostID := s.Node.ID
 
 	_, getOutput, err := s.ExecuteTestCobraCommand("job", "get",
 		"--ipfs-connect", s.Config.Node.IPFS.Connect,
@@ -223,7 +82,7 @@ func (s *GetSuite) TestGetSingleFileFromOutput() {
 	require.NoError(s.T(), err, "Error getting results")
 
 	testDownloadOutput(s.T(), getOutput, jobID, filepath.Join(tempDir, util.GetDefaultJobFolder(jobID)))
-	testResultsFolderStructure(s.T(), filepath.Join(tempDir, util.GetDefaultJobFolder(jobID)), []string{"/stdout"})
+	testResultsFolderStructure(s.T(), filepath.Join(tempDir, util.GetDefaultJobFolder(jobID)), hostID, []string{"/stdout"})
 }
 
 func (s *GetSuite) TestGetSingleNestedFileFromOutput() {
@@ -237,7 +96,8 @@ func (s *GetSuite) TestGetSingleNestedFileFromOutput() {
 	})
 	_, out, err := s.ExecuteTestCobraCommand(args...)
 	require.NoError(s.T(), err, "Error submitting job")
-	jobID := system.FindJobIDInTestOutputLegacy(out)
+	jobID := system.FindJobIDInTestOutput(out)
+	hostID := s.Node.ID
 
 	_, getOutput, err := s.ExecuteTestCobraCommand("job", "get",
 		"--ipfs-connect", s.Config.Node.IPFS.Connect,
@@ -250,6 +110,7 @@ func (s *GetSuite) TestGetSingleNestedFileFromOutput() {
 	testDownloadOutput(s.T(), getOutput, jobID, filepath.Join(tempDir, util.GetDefaultJobFolder(jobID)))
 	testResultsFolderStructure(s.T(),
 		filepath.Join(tempDir, util.GetDefaultJobFolder(jobID)),
+		hostID,
 		[]string{
 			"/data",
 			"/data/apples",
@@ -271,7 +132,8 @@ func (s *GetSuite) TestGetWriteToJobFolderNamedDownload() {
 	_, out, err := s.ExecuteTestCobraCommand(args...)
 
 	require.NoError(s.T(), err, "Error submitting job")
-	jobID := system.FindJobIDInTestOutputLegacy(out)
+	jobID := system.FindJobIDInTestOutput(out)
+	hostID := s.Node.ID
 
 	_, getOutput, err := s.ExecuteTestCobraCommand("job", "get",
 		"--output-dir", tempDir,
@@ -279,5 +141,82 @@ func (s *GetSuite) TestGetWriteToJobFolderNamedDownload() {
 	)
 	require.NoError(s.T(), err, "Error getting results")
 	testDownloadOutput(s.T(), getOutput, jobID, tempDir)
-	testResultsFolderStructure(s.T(), tempDir, nil)
+	testResultsFolderStructure(s.T(), tempDir, hostID, nil)
+}
+
+func testResultsFolderStructure(t *testing.T, baseFolder, hostID string, expectedFiles []string) {
+	var files []string
+	err := filepath.Walk(baseFolder, func(path string, _ os.FileInfo, _ error) error {
+		usePath := strings.Replace(path, baseFolder, "", 1)
+		if usePath != "" {
+			files = append(files, usePath)
+		}
+		return nil
+	})
+	require.NoError(t, err, "Error walking results directory")
+
+	var expected []string
+	if expectedFiles != nil {
+		expected = expectedFiles
+	} else {
+		// Default folder structure if nothing was provided
+		expected = []string{
+			"/data",
+			"/data/apples",
+			"/data/apples/file.txt",
+			"/data/file.txt",
+			"/exitCode",
+			"/outputs",
+			"/" + downloader.DownloadFilenameStderr,
+			"/" + downloader.DownloadFilenameStdout,
+		}
+	}
+
+	require.Equal(t, strings.Join(expected, "\n"), strings.Join(files, "\n"), "The discovered results output structure was not correct")
+}
+
+func testDownloadOutput(t *testing.T, cmdOutput, jobID, outputDir string) {
+	require.True(t, strings.Contains(
+		cmdOutput,
+		fmt.Sprintf("Results for job '%s'", jobID),
+	), "Job ID not found in output")
+	require.True(t, strings.Contains(
+		cmdOutput,
+		outputDir,
+	), "Download location not found in output")
+
+}
+
+func setupTempWorkingDir(t *testing.T) (string, func()) {
+	// switch wd to a temp dir so we are not writing folders to the current directory
+	// (the point of this test is to see what happens when we DON'T pass --output-dir)
+	tempDir := t.TempDir()
+	originalWd, err := os.Getwd()
+	require.NoError(t, err)
+	err = os.Chdir(tempDir)
+	require.NoError(t, err)
+	// Below looks redundant but it is necessary on Darwin which does some
+	// freaky symlinking of root dirs: https://unix.stackexchange.com/a/63565
+	newTempDir, err := os.Getwd()
+	require.NoError(t, err)
+	return newTempDir, func() {
+		assert.NoError(t, os.Chdir(originalWd))
+	}
+}
+
+func (s *GetSuite) getDockerRunArgs(extraArgs []string) []string {
+	args := []string{
+		"docker", "run",
+		"--publisher", "local",
+		"-o", "data:/data",
+		"--wait",
+	}
+	args = append(args, extraArgs...)
+	args = append(args,
+		"ubuntu:kinetic",
+		"--",
+		"bash", "-c",
+		"echo hello > /data/file.txt && echo hello && mkdir /data/apples && echo oranges > /data/apples/file.txt",
+	)
+	return args
 }
diff --git a/test/get-results.sh b/test/get-results.sh
index 355925ec65..7f159a61c0 100644
--- a/test/get-results.sh
+++ b/test/get-results.sh
@@ -7,7 +7,7 @@ testcase_can_get_results() {
     create_node requester,compute
 
     job_id=$(bacalhau job run --id-only $ROOT/testdata/jobs/docker-output.yaml)
-    bacalhau get $job_id > /dev/null 2>&1
+    bacalhau job get $job_id > /dev/null 2>&1
     subject cat job-*/output_custom/output.txt
     assert_equal 0 $status
     assert_match "15" $(echo $stdout)