Make call.End more solid

.circleci/config.yml

@@ -33,7 +33,6 @@ jobs:
       # login here for tests
       - run: docker login -u $DOCKER_USER -p $DOCKER_PASS
       - run: ./test.sh
-      - run: ./go-fmt.sh
       - deploy:
           command: |
             if [ "${CIRCLE_BRANCH}" == "master" ]; then

api/agent/agent.go

@@ -224,7 +224,9 @@ func (a *agent) Submit(callI Call) error {
 
 	// TODO if the context is timed out here we need to allocate some more time...
 	// right now this only works b/c the db isn't using the context
-	return call.End(ctx, err)
+	call.End(ctx, err)
+
+	return err
 }
 
 // getSlot must ensure that if it receives a slot, it will be returned, otherwise

api/agent/call.go

@@ -35,7 +35,7 @@ type Call interface {
 	// regardless of whether the execution failed or not. An error will be passed
 	// to End, which if nil indicates a successful execution. Any error returned
 	// from End will be returned as the error from Submit.
-	End(ctx context.Context, err error) error
+	End(ctx context.Context, err error)
 }
 
 // TODO build w/o closures... lazy
@@ -267,7 +267,7 @@ func (c *call) Start(ctx context.Context) error {
 	return nil
 }
 
-func (c *call) End(ctx context.Context, err error) error {
+func (c *call) End(ctx context.Context, err error) {
 	span, ctx := opentracing.StartSpanFromContext(ctx, "agent_call_end")
 	defer span.Finish()
 
@@ -287,19 +287,13 @@ func (c *call) End(ctx context.Context, err error) error {
 		// XXX (reed): delete MQ message, eventually
 	}
 
-	// TODO since the function itself can reply directly to a client (or logs),
+	// need newer context because original one may be modified
-	// this means that we could potentially store an error / timeout status for a
+	// and no longer be valid for further context-bound operations
-	// call that ran successfully [by a user's perspective]
+	if err := c.ds.InsertCall(context.Background(), c.Call); err != nil {
-	// TODO this should be update, really
+		// TODO we should just log this error not return it to user?
-	if err := c.ds.InsertCall(ctx, c.Call); err != nil {
+		// just issue storing call status but call is run
-		// TODO we should just log this error not return it to user? just issue storing call status but call is run
 		logrus.WithError(err).Error("error inserting call into datastore")
 	}
-
-	// return the original error so that this is returned from Submit (for sync)
-	// TODO we could just skip over (and log) and End errors and return slot.exec error
-	// in submit instead of doing this, it's a bit odd. thoughts?
-	return err
 }
 
 func (a *agent) route(ctx context.Context, appName, path string) (*models.Route, error) {

go-fmt.sh

@@ -1,7 +1,7 @@
 #! /bin/sh
 set -e
 
-function listFilesExit() {
+listFilesExit () {
 	echo The following files need to have go fmt ran:
 	echo $NEED_TO_FORMAT
 	exit 1