From 085bc39c502b08f5104b3bd01569bd56222eb9bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A1n=20Del=20Valle?= <commit@ridv.xyz>
Date: Fri, 30 Apr 2021 11:17:52 -0700
Subject: [PATCH] Breaking change for CreateJob

Create job has changed from a tuple return to a single error return
which simplifies how we interact with the api.
---
 examples/client.go |  9 +++------
 realis.go          | 11 ++++++-----
 retry.go           |  6 +++---
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/examples/client.go b/examples/client.go
index 69e3751..c57a9fe 100644
--- a/examples/client.go
+++ b/examples/client.go
@@ -166,11 +166,10 @@ func main() {
 	switch cmd {
 	case "create":
 		fmt.Println("Creating job")
-		resp, err := r.CreateJob(job)
+		err := r.CreateJob(job)
 		if err != nil {
 			log.Fatalln(err)
 		}
-		fmt.Println(resp.String())
 
 		if ok, mErr := monitor.Instances(job.JobKey(), job.GetInstanceCount(), 5, 50); !ok || mErr != nil {
 			_, err := r.KillJob(job.JobKey())
@@ -205,11 +204,10 @@ func main() {
 		fmt.Println("Creating a docker based job")
 		container := realis.NewDockerContainer().Image("python:2.7").AddParameter("network", "host")
 		job.Container(container)
-		resp, err := r.CreateJob(job)
+		err := r.CreateJob(job)
 		if err != nil {
 			log.Fatal(err)
 		}
-		fmt.Println(resp.String())
 
 		if ok, err := monitor.Instances(job.JobKey(), job.GetInstanceCount(), 10, 300); !ok || err != nil {
 			_, err := r.KillJob(job.JobKey())
@@ -222,11 +220,10 @@ func main() {
 		fmt.Println("Creating a docker based job")
 		container := realis.NewMesosContainer().DockerImage("python", "2.7")
 		job.Container(container)
-		resp, err := r.CreateJob(job)
+		err := r.CreateJob(job)
 		if err != nil {
 			log.Fatal(err)
 		}
-		fmt.Println(resp.String())
 
 		if ok, err := monitor.Instances(job.JobKey(), job.GetInstanceCount(), 10, 300); !ok || err != nil {
 			_, err := r.KillJob(job.JobKey())
diff --git a/realis.go b/realis.go
index 07385d5..e9ea136 100644
--- a/realis.go
+++ b/realis.go
@@ -44,7 +44,7 @@ const version = "1.23.1"
 type Realis interface {
 	AbortJobUpdate(updateKey aurora.JobUpdateKey, message string) (*aurora.Response, error)
 	AddInstances(instKey aurora.InstanceKey, count int32) (*aurora.Response, error)
-	CreateJob(auroraJob Job) (*aurora.Response, error)
+	CreateJob(auroraJob Job) error
 	CreateService(
 		auroraJob Job,
 		settings *aurora.JobUpdateSettings) (*aurora.Response, *aurora.StartJobUpdateResult_, error)
@@ -663,11 +663,12 @@ func (r *realisClient) KillJob(key *aurora.JobKey) (*aurora.Response, error) {
 // Although this API is able to create service jobs, it is better to use CreateService instead
 // as that API uses the update thrift call which has a few extra features available.
 // Use this API to create ad-hoc jobs.
-func (r *realisClient) CreateJob(auroraJob Job) (*aurora.Response, error) {
+func (r *realisClient) CreateJob(auroraJob Job) error {
 
 	r.logger.debugPrintf("CreateJob Thrift Payload: %+v\n", auroraJob.JobConfig())
 
-	resp, retryErr := r.thriftCallWithRetries(
+	// Response is checked by the thrift retry code
+	_, retryErr := r.thriftCallWithRetries(
 		false,
 		func() (*aurora.Response, error) {
 			return r.client.CreateJob(context.TODO(), auroraJob.JobConfig())
@@ -692,9 +693,9 @@ func (r *realisClient) CreateJob(auroraJob Job) (*aurora.Response, error) {
 	)
 
 	if retryErr != nil {
-		return resp, errors.Wrap(retryErr, "error sending Create command to Aurora Scheduler")
+		return errors.Wrap(retryErr, "error sending Create command to Aurora Scheduler")
 	}
-	return resp, nil
+	return nil
 }
 
 // CreateService uses the scheduler's updating mechanism to create a job.
diff --git a/retry.go b/retry.go
index 1ecf467..c7b3ef6 100644
--- a/retry.go
+++ b/retry.go
@@ -167,7 +167,7 @@ func (r *realisClient) thriftCallWithRetries(
 			// Print out the error to the user
 			r.logger.Printf("Client Error: %v", clientErr)
 
-			temporary, timedout := processClientError(clientErr)
+			temporary, timedout := isConnectionError(clientErr)
 			if !temporary && r.RealisConfig().failOnPermanentErrors {
 				return nil, errors.Wrap(clientErr, "permanent connection error")
 			}
@@ -257,10 +257,10 @@ func (r *realisClient) thriftCallWithRetries(
 	return nil, newRetryError(errors.New("ran out of retries"), curStep)
 }
 
-// processClientError processes the error received by the client.
+// isConnectionError processes the error received by the client.
 // The return values indicate weather this was determined to be a temporary error
 // and weather it was determined to be a timeout error
-func processClientError(err error) (bool, bool) {
+func isConnectionError(err error) (bool, bool) {
 
 	// Determine if error is a temporary URL error by going up the stack
 	transportException, ok := err.(thrift.TTransportException)