Milo: Fix project importer so it doesn't error out on a bad config

milo/common/config.go

 // Copyright 2016 The LUCI Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //      http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 package common
 
 import (
         "fmt"
         "time"
 
         "github.com/golang/protobuf/proto"
         "golang.org/x/net/context"
 
         "github.com/luci/gae/service/datastore"
         "github.com/luci/gae/service/info"
         "github.com/luci/luci-go/common/data/caching/proccache"
+        "github.com/luci/luci-go/common/errors"
         "github.com/luci/luci-go/common/logging"
         "github.com/luci/luci-go/luci_config/server/cfgclient"
         "github.com/luci/luci-go/luci_config/server/cfgclient/backend"
         "github.com/luci/luci-go/luci_config/server/cfgclient/textproto"
 
         "github.com/luci/luci-go/milo/api/config"
 )
 
 // Project is a LUCI project.
 type Project struct {
-        // The ID of the project, as per self defined.  This is not the luci-config
-        // name.
+        // ID of the project, as per self defined.  This is the luci-config name.
         ID string `gae:"$id"`
-        // The luci-config name of the project.
-        Name string
-        // The Project data in protobuf binary format.
+        // Data is the Project data in protobuf binary format.
         Data []byte `gae:",noindex"`
+        // Revision is the latest revision we have of this project's config.
+        Revision string
 }
 
 // The key for the service config entity in datastore.
 const ServiceConfigID = "service_config"
 
 // ServiceConfig is a container for the instance's service config.
 type ServiceConfig struct {
         // ID is the datastore key.  This should be static, as there should only be
         // one service config.
         ID string `gae:"$id"`
@@ skipping 126 matching lines @@
 
 // UpdateProjectConfigs internal project configuration based off luci-config.
 // update updates Milo's configuration based off luci config.  This includes
 // scanning through all project and extract all console configs.
 func UpdateProjectConfigs(c context.Context) error {
         cfgName := info.AppID(c) + ".cfg"
 
         var (
                 configs []*config.Project
                 metas   []*cfgclient.Meta
+                merr    errors.MultiError
         )
+
         logging.Debugf(c, "fetching configs for %s", cfgName)
         if err := cfgclient.Projects(c, cfgclient.AsService, cfgName, textproto.Slice(&configs), &metas); err != nil {
                 logging.WithError(err).Errorf(c, "Encountered error while getting project config for %s", cfgName)

[iannucci, 2017/07/15 01:23:14]

I would skip this since you log in the loop below.

[Ryan Tseng, 2017/07/15 01:24:24]

Done.

-                return err
+                merr = err.(errors.MultiError)
+                // We can continue with the ones that work still.
         }
 
         // A map of project ID to project.
         projects := map[string]*Project{}
         for i, proj := range configs {
-                projectName, _, _ := metas[i].ConfigSet.SplitProject()
+                meta := metas[i]
+                projectName, _, _ := meta.ConfigSet.SplitProject()
+                name := string(projectName)
+                projects[name] = nil
+                if merr != nil && merr[i] != nil {
+                        logging.WithError(merr[i]).Warningf(c, "skipping %s due to error", name)
+                        continue
+                }
 
-                logging.Infof(c, "Prossing %s", projectName)
-                if dup, ok := projects[proj.ID]; ok {
-                        return fmt.Errorf(
-                                "Duplicate project ID: %s. (%s and %s)", proj.ID, dup.Name, projectName)
+                p := &Project{
+                        ID:       name,
+                        Revision: meta.Revision,
                 }
-                p := &Project{
-                        ID:   proj.ID,
-                        Name: string(projectName),
-                }
-                projects[proj.ID] = p
+
+                logging.Infof(c, "prossing %s", name)
 
                 var err error
                 p.Data, err = proto.Marshal(proj)
                 if err != nil {
-                        return err
+                        logging.WithError(err).Errorf(c, "Encountered error while processing project %s", name)
+                        // Set this to nil to signal this project exists but we don't want to update it.
+                        projects[name] = nil
+                        continue
                 }
+                projects[name] = p
         }
 
         // Now load all the data into the datastore.
         projs := make([]*Project, 0, len(projects))
         for _, proj := range projects {
-                projs = append(projs, proj)
+                if proj != nil {
+                        projs = append(projs, proj)
+                }
         }
         if err := datastore.Put(c, projs); err != nil {
                 return err
         }
 
         // Delete entries that no longer exist.
         q := datastore.NewQuery("Project").KeysOnly(true)
         allProjs := []Project{}
         datastore.GetAll(c, q, &allProjs)
         toDelete := []Project{}
         for _, proj := range allProjs {
                 if _, ok := projects[proj.ID]; !ok {
                         toDelete = append(toDelete, proj)
                 }
         }
-        datastore.Delete(c, toDelete)
-
-        return nil
+        return datastore.Delete(c, toDelete)
 }
 
 // GetAllProjects returns all registered projects.
 func GetAllProjects(c context.Context) ([]*config.Project, error) {
         q := datastore.NewQuery("Project")
         q.Order("ID")
 
         ps := []*Project{}
         err := datastore.GetAll(c, q, &ps)
         if err != nil {
@@ skipping 30 matching lines @@
         if err != nil {
                 return nil, err
         }
         for _, cs := range p.Consoles {
                 if cs.Name == consoleName {
                         return cs, nil
                 }
         }
         return nil, fmt.Errorf("Console %s not found in project %s", consoleName, projName)
 }