review comments

clients/manifest.go

@@ -169,6 +169,9 @@ func downloadManifest(requestID, sourceManifestOSURL string) (playlist m3u8.Play
 		rc, err := GetFile(context.Background(), requestID, sourceManifestOSURL, dStorage)
 		if err != nil {
 			if time.Since(start) > 10*time.Second && errors.IsObjectNotFound(err) {
+				// bail out of the retries earlier for not found errors because it will be quite a common scenario
+				// where the backup manifest does not exist and we don't want to wait the whole 50s of retries for
+				// every recording job
 				return backoff.Permanent(err)
 			}
 			return err

config/storage_backup_url_test.go

@@ -0,0 +1,30 @@
+package config
+
+import "testing"
+
+func TestGetStorageBackupURL(t *testing.T) {
+	StorageFallbackURLs = map[string]string{"https://storj.livepeer.com/catalyst-recordings-com/hls": "https://google.livepeer.com/catalyst-recordings-com/hls"}
+	tests := []struct {
+		name   string
+		urlStr string
+		want   string
+	}{
+		{
+			name:   "should replace",
+			urlStr: "https://storj.livepeer.com/catalyst-recordings-com/hls/foo",
+			want:   "https://google.livepeer.com/catalyst-recordings-com/hls/foo",
+		},
+		{
+			name:   "should not replace",
+			urlStr: "https://blah.livepeer.com/catalyst-recordings-com/hls/foo",
+			want:   "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := GetStorageBackupURL(tt.urlStr); got != tt.want {
+				t.Errorf("GetStorageBackupURL() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

main.go

@@ -117,7 +117,7 @@ func main() {
 	fs.StringVar(&cli.EncryptKey, "encrypt", "", "Key for encrypting network traffic within Serf. Must be a base64-encoded 32-byte key.")
 	fs.StringVar(&cli.VodDecryptPublicKey, "catalyst-public-key", "", "Public key of the catalyst node for encryption")
 	fs.StringVar(&cli.VodDecryptPrivateKey, "catalyst-private-key", "", "Private key of the catalyst node for encryption")
-	config.CommaMapFlag(fs, &cli.StorageFallbackURLs, "storage-fallback-urls", map[string]string{}, `Comma-separated map of primary to backup storage URLs. If a file fails downloading from one of the primary storages (detected by prefix), it will fallback to the corresponding backup URL after having the prefix replaced`)
+	config.CommaMapFlag(fs, &cli.StorageFallbackURLs, "storage-fallback-urls", map[string]string{}, `Comma-separated map of primary to backup storage URLs. If a file fails downloading from one of the primary storages (detected by prefix), it will fallback to the corresponding backup URL after having the prefix replaced. E.g. https://storj.livepeer.com/catalyst-recordings-com/hls=https://google.livepeer.com/catalyst-recordings-com/hls`)
 	fs.StringVar(&cli.GateURL, "gate-url", "http://localhost:3004/api/access-control/gate", "Address to contact playback gating API for access control verification")
 	fs.StringVar(&cli.DataURL, "data-url", "http://localhost:3004/api/data", "Address of the Livepeer Data Endpoint")
 	config.InvertedBoolFlag(fs, &cli.MistTriggerSetup, "mist-trigger-setup", true, "Overwrite Mist triggers with the ones built into catalyst-api")

test/steps/ffmpeg.go

@@ -31,19 +31,20 @@ func (s *StepContext) AllOfTheSourceSegmentsAreWrittenToStorageWithinSeconds(num
 	session := osDriver.NewSession(filepath.Join(s.latestRequestID, "source"))
 
 	var latestNumSegments int
-	for x := 0; x < secs; x++ {
+	for x := 0; x < secs; x++ { // retry loop
+		if x > 0 {
+			time.Sleep(time.Second)
+		}
 		page, err := session.ListFiles(context.Background(), "", "")
 		if err != nil {
 			log.Println("failed to list files: ", err)
-			time.Sleep(time.Second)
 			continue
 		}
 
 		latestNumSegments = len(page.Files())
 		if latestNumSegments == numSegments+1 {
 			return nil
 		}
-		time.Sleep(time.Second)
 	}
 	return fmt.Errorf("did not find the expected number of source segments in %s (wanted %d, got %d)", s.SourceOutputDir, numSegments, latestNumSegments)
 }