Clean up usage of Name fields in job spec

These are legacy and no longer needed as we now understand how to
correctly unmarshal enum types.

Also, commands should return errors even when calling Fatal, so that
the error condition can be probably caught during tests.

cmd/bacalhau/create.go

@@ -98,7 +98,7 @@ var createCmd = &cobra.Command{
 			byteResult, err = ReadFromStdinIfAvailable(cmd, cmdArgs)
 			if err != nil {
 				Fatal(fmt.Sprintf("Unknown error reading from file or stdin: %s\n", err), 1)
-				return nil
+				return err
 			}
 		} else {
 			OC.Filename = cmdArgs[0]
@@ -108,34 +108,34 @@ var createCmd = &cobra.Command{
 
 			if err != nil {
 				Fatal(fmt.Sprintf("Error opening file: %s", err), 1)
-				return nil
+				return err
 			}
 
 			byteResult, err = io.ReadAll(fileContent)
 			if err != nil {
 				Fatal(fmt.Sprintf("Error reading file: %s", err), 1)
-				return nil
+				return err
 			}
 		}
 
 		// Do a first pass for parsing to see if it's a Job or JobWithInfo
 		err = yaml.Unmarshal(byteResult, &rawMap)
 		if err != nil {
 			Fatal(fmt.Sprintf("Error parsing file: %s", err), 1)
-			return nil
+			return err
 		}
 
 		// If it's a JobWithInfo, we need to convert it to a Job
 		if _, isJobWithInfo := rawMap["Job"]; isJobWithInfo {
 			err = yaml.Unmarshal(byteResult, &jwi)
 			if err != nil {
 				Fatal(userstrings.JobSpecBad, 1)
-				return nil
+				return err
 			}
 			byteResult, err = yaml.Marshal(jwi.Job)
 			if err != nil {
 				Fatal(userstrings.JobSpecBad, 1)
-				return nil
+				return err
 			}
 		}
 
@@ -144,13 +144,13 @@ var createCmd = &cobra.Command{
 		err = yaml.Unmarshal(byteResult, &j)
 		if err != nil {
 			Fatal(userstrings.JobSpecBad, 1)
-			return nil
+			return err
 		}
 
 		// See if the job spec is empty
 		if j == nil || reflect.DeepEqual(j.Spec, &model.Job{}) {
 			Fatal(userstrings.JobSpecBad, 1)
-			return nil
+			return err
 		}
 
 		// Warn on fields with data that will be ignored
@@ -201,10 +201,10 @@ var createCmd = &cobra.Command{
 		if err != nil {
 			if _, ok := err.(*bacerrors.ImageNotFound); ok {
 				Fatal(fmt.Sprintf("Docker image '%s' not found in the registry, or needs authorization.", j.Spec.Docker.Image), 1)
-				return nil
+				return err
 			} else {
 				Fatal(fmt.Sprintf("Error verifying job: %s", err), 1)
-				return nil
+				return err
 			}
 		}
 		if ODR.DryRun {
@@ -213,7 +213,7 @@ var createCmd = &cobra.Command{
 			yamlBytes, err = yaml.Marshal(j)
 			if err != nil {
 				Fatal(fmt.Sprintf("Error converting job to yaml: %s", err), 1)
-				return nil
+				return err
 			}
 			cmd.Print(string(yamlBytes))
 			return nil
@@ -230,6 +230,7 @@ var createCmd = &cobra.Command{
 
 		if err != nil {
 			Fatal(fmt.Sprintf("Error executing job: %s", err), 1)
+			return err
 		}
 
 		return nil

cmd/bacalhau/describe.go

@@ -80,12 +80,8 @@ var describeCmd = &cobra.Command{
 			byteResult, err = ReadFromStdinIfAvailable(cmd, cmdArgs)
 			// If there's no input ond no stdin, then cmdArgs is nil, and byteResult is nil.
 			if err != nil {
-				if err != nil {
-					Fatal(fmt.Sprintf("Unknown error reading from file or stdin: %s\n", err), 1)
-					return nil
-				}
-				// Error not related to fields being empty
 				Fatal(fmt.Sprintf("Unknown error reading from file: %s\n", err), 1)
+				return err
 			}
 			inputJobID = string(byteResult)
 		}

cmd/bacalhau/get.go

@@ -71,12 +71,8 @@ var getCmd = &cobra.Command{
 			var byteResult []byte
 			byteResult, err = ReadFromStdinIfAvailable(cmd, cmdArgs)
 			if err != nil {
-				if err != nil {
-					Fatal(fmt.Sprintf("Unknown error reading from file or stdin: %s\n", err), 1)
-					return nil
-				}
-				// Error not related to fields being empty
 				Fatal(fmt.Sprintf("Unknown error reading from file: %s\n", err), 1)
+				return err
 			}
 			jobID = string(byteResult)
 		}
@@ -92,11 +88,13 @@ var getCmd = &cobra.Command{
 			} else {
 				Fatal(fmt.Sprintf("Unknown error trying to get job (ID: %s): %+v", jobID, err), 1)
 			}
+			return err
 		}
 
 		results, err := GetAPIClient().GetResults(ctx, j.ID)
 		if err != nil {
 			Fatal(fmt.Sprintf("Error getting results for job ID (%s): %s", jobID, err), 1)
+			return err
 		}
 
 		err = ipfs.DownloadJob(
@@ -109,6 +107,7 @@ var getCmd = &cobra.Command{
 
 		if err != nil {
 			Fatal(fmt.Sprintf("Error downloading results from job ID (%s): %s", jobID, err), 1)
+			return err
 		}
 
 		return nil

pkg/model/engine.go

@@ -33,13 +33,6 @@ func ParseEngine(str string) (Engine, error) {
 		"executor: unknown engine type '%s'", str)
 }
 
-func EnsureEngine(typ Engine, str string) (Engine, error) {
-	if IsValidEngine(typ) {
-		return typ, nil
-	}
-	return ParseEngine(str)
-}
-
 func EngineTypes() []Engine {
 	var res []Engine
 	for typ := engineUnknown + 1; typ < engineDone; typ++ {

pkg/model/job.go

@@ -190,19 +190,13 @@ type Deal struct {
 // Spec is a complete specification of a job that can be run on some
 // execution provider.
 type Spec struct {
-	// TODO: #643 #642 Merge EngineType & Engine, VerifierType & VerifierName, Publisher & PublisherName - this seems like an issue
 	// e.g. docker or language
 	Engine Engine `json:"Engine,omitempty"`
-	// allow the engine to be provided as a string for JSON job specs
-	EngineName string `json:"EngineName,omitempty"`
 
 	Verifier Verifier `json:"Verifier,omitempty"`
-	// allow the verifier to be provided as a string for JSON job specs
-	VerifierName string `json:"VerifierName,omitempty"`
 
 	// there can be multiple publishers for the job
-	Publisher     Publisher `json:"Publisher,omitempty"`
-	PublisherName string    `json:"PublisherName,omitempty"`
+	Publisher Publisher `json:"Publisher,omitempty"`
 
 	// executor specific data
 	Docker   JobSpecDocker   `json:"Docker,omitempty"`

pkg/model/publisher.go

@@ -26,13 +26,6 @@ func ParsePublisher(str string) (Publisher, error) {
 	return publisherUnknown, fmt.Errorf("verifier: unknown type '%s'", str)
 }
 
-func EnsurePublisher(typ Publisher, str string) (Publisher, error) {
-	if IsValidPublisher(typ) {
-		return typ, nil
-	}
-	return ParsePublisher(str)
-}
-
 func IsValidPublisher(publisherType Publisher) bool {
 	return publisherType > publisherUnknown && publisherType < publisherDone
 }

pkg/model/storage_source.go

@@ -33,34 +33,6 @@ func ParseStorageSourceType(str string) (StorageSourceType, error) {
 		"executor: unknown source type '%s'", str)
 }
 
-func EnsureStorageSourceType(typ StorageSourceType, str string) (StorageSourceType, error) {
-	if IsValidStorageSourceType(typ) {
-		return typ, nil
-	}
-	return ParseStorageSourceType(str)
-}
-
-func EnsureStorageSpecSourceType(spec StorageSpec) (StorageSpec, error) {
-	engine, err := EnsureStorageSourceType(spec.StorageSource, spec.StorageSourceName)
-	if err != nil {
-		return spec, err
-	}
-	spec.StorageSource = engine
-	return spec, nil
-}
-
-func EnsureStorageSpecsSourceTypes(specs []StorageSpec) ([]StorageSpec, error) {
-	ret := []StorageSpec{}
-	for _, spec := range specs {
-		newSpec, err := EnsureStorageSpecSourceType(spec)
-		if err != nil {
-			return ret, err
-		}
-		ret = append(ret, newSpec)
-	}
-	return ret, nil
-}
-
 func IsValidStorageSourceType(sourceType StorageSourceType) bool {
 	return sourceType > storageSourceUnknown && sourceType < storageSourceDone
 }

pkg/model/storage_spec.go

@@ -4,10 +4,10 @@ package model
 // specific to particular execution engines, as different execution engines
 // will mount data in different ways.
 type StorageSpec struct {
-	// TODO: #645 Is this engine name the same as the Job EngineName?
-	// StorageSource is the execution engine that can mount the spec's data.
-	StorageSource     StorageSourceType `json:"StorageSource,omitempty"`
-	StorageSourceName string            `json:"StorageSourceName,omitempty"`
+	// StorageSource is the abstract source of the data. E.g. a storage source
+	// might be a URL download, but doesn't specify how the execution engine
+	// does the download or what it will do with the downloaded data.
+	StorageSource StorageSourceType `json:"StorageSource,omitempty"`
 
 	// Name of the spec's data, for reference.
 	Name string `json:"Name,omitempty"`

pkg/model/verifier.go

@@ -24,13 +24,6 @@ func ParseVerifier(str string) (Verifier, error) {
 	return verifierUnknown, fmt.Errorf("verifier: unknown type '%s'", str)
 }
 
-func EnsureVerifier(typ Verifier, str string) (Verifier, error) {
-	if IsValidVerifier(typ) {
-		return typ, nil
-	}
-	return ParseVerifier(str)
-}
-
 func IsValidVerifier(verifierType Verifier) bool {
 	return verifierType > verifierUnknown && verifierType < verifierDone
 }