Milo: Pending swarming builds

appengine/cmd/milo/swarming/build.go

 // Copyright 2015 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package swarming
 
 import (
         "bytes"
         "encoding/json"
         "fmt"
@@ skipping 96 matching lines @@
         *swarming.SwarmingRpcsTaskResult, error) {
 
         trc := sc.Task.Result(taskID)
         srtr, err := trc.Do()
         if err != nil {
                 return nil, err
         }
         return srtr, nil
 }
 
+// getSwarming fetches both the swarming task data and log data, and returns
+// the task result, the log string, task error, and log error.
 func getSwarming(c context.Context, server string, taskID string) (
-        *swarming.SwarmingRpcsTaskResult, string, error) {
+        *swarming.SwarmingRpcsTaskResult, string, error, error) {
 
         var log string
         var sr *swarming.SwarmingRpcsTaskResult
         var errLog, errRes error
 
         // Detour: Return debugging results, useful for development.
         if server == "debug" {
                 sr, errRes = getDebugSwarmingResult(taskID)
                 log, errLog = getDebugTaskOutput(taskID)
-                if errLog != nil {
-                        return sr, log, errLog
-                }
-                return sr, log, errRes
+                return sr, log, errRes, errLog
         }
 
         sc, err := getSwarmingClient(c, server)
         if err != nil {
-                return nil, "", err
+                return nil, "", err, nil
         }
 
         var wg sync.WaitGroup
         wg.Add(2) // Getting log and result can happen concurrently.  Wait for both.
 
         go func() {
                 defer wg.Done()
                 log, errLog = getTaskOutput(sc, taskID)
         }()
         go func() {
                 defer wg.Done()
                 sr, errRes = getSwarmingResult(sc, taskID)
         }()
         wg.Wait()
-        if errRes != nil {
-                return sr, log, errRes
-        }
-        return sr, log, errLog
+        return sr, log, errRes, errLog
 }
 
 func taskProperties(sr *swarming.SwarmingRpcsTaskResult) *resp.PropertyGroup {
         props := &resp.PropertyGroup{GroupName: "Swarming"}
         if len(sr.CostsUsd) == 1 {
                 props.Property = append(props.Property, &resp.Property{
                         Key:   "Cost of job (USD)",
                         Value: fmt.Sprintf("$%.2f", sr.CostsUsd[0]),
                 })
         }
@@ skipping 120 matching lines @@
         // goroutine safe
         if err := p.RunStreams([]*annotee.Stream{&is}); err != nil {
                 return nil, err
         }
         p.Finish()
         return c.ToLogDogStreams()
 }
 
 func swarmingBuildImpl(c context.Context, URL string, server string, taskID string) (*resp.MiloBuild, error) {
         // Fetch the data from Swarming
-        sr, body, err := getSwarming(c, server, taskID)
-        if err != nil {
-                return nil, err
+        sr, body, errRes, errLog := getSwarming(c, server, taskID)
+        // Swarming result must come back.
+        if errRes != nil {
+                return nil, errRes
+        }
+        switch sr.State {
+        case TaskCompleted, TaskRunning, TaskCanceled:
+                if errLog != nil {
+                        // If the task is completed, running, or canceled, we'd expect a log.
+                        // If not, we don't necessarily expect the log to come back.

[nodir, 2016/06/28 20:36:49]

I think getSwarming is a better place for this logic (since perhaps all users of
getSwarming will have to replicate it)
this would simplify getSwarming func signature too (won't have to have two
errors)

+                        return nil, errLog
+                }
         }
 
         allowMilo := false
         for _, t := range sr.Tags {
                 if t == "allow_milo:1" {
                         allowMilo = true
                         break
                 }
         }
         if !allowMilo {
                 return nil, fmt.Errorf("Not A Milo Job")
         }
 
         build, err := taskToBuild(c, server, sr)
         if err != nil {
                 return nil, err
         }
 
         // Decode the data using annotee. The logdog stream returned here is assumed
         // to be consistent, which is why the following block of code are not
         // expected to ever err out.
-        lds, err := streamsFromAnnotatedLog(c, body)
-        if err != nil {
-                build.Components = []*resp.BuildComponent{{
-                        Type:   resp.Summary,
-                        Label:  "Milo annotation parser",
-                        Text:   []string{err.Error()},
-                        Status: resp.InfraFailure,
-                        SubLink: []*resp.Link{{
-                                Label: "swarming task",
-                                URL:   taskPageURL(server, taskID),
-                        }},
-                }}
-        } else {
-                logdog.AddLogDogToBuild(c, URL, lds, build)
+        if errLog != nil && body != "" {
+                lds, err := streamsFromAnnotatedLog(c, body)
+                if err != nil {
+                        build.Components = []*resp.BuildComponent{{
+                                Type:   resp.Summary,
+                                Label:  "Milo annotation parser",
+                                Text:   []string{err.Error()},
+                                Status: resp.InfraFailure,
+                                SubLink: []*resp.Link{{
+                                        Label: "swarming task",
+                                        URL:   taskPageURL(server, taskID),
+                                }},
+                        }}
+                } else {
+                        logdog.AddLogDogToBuild(c, URL, lds, build)
+                }
         }
 
         return build, nil
 }
 
 // taskPageURL returns a URL to a human-consumable page of a swarming task.
 // Supports server aliases.
 func taskPageURL(swarmingHostname, taskID string) string {
         return fmt.Sprintf("https://%s/user/task/%s", resolveServer(swarmingHostname), taskID)
 }
 
 // botPageURL returns a URL to a human-consumable page of a swarming bot.
 // Supports server aliases.
 func botPageURL(swarmingHostname, botID string) string {
         return fmt.Sprintf("https://%s/restricted/bot/%s", resolveServer(swarmingHostname), botID)
 }