app/appengine: fix up mistakes from not test deploying earlier CLs
During my earlier deletion spree over the past few CLs I wasn't
testing on real data (with production data) and didn't catch that
datastore returns an error when you load an old entity into a struct
that has since removed a field.
So filter those out.
(I could've also restored those fields, perhaps with a dummy named
interface{} type, but this code is temporary, and I'd prefer fewer
fields to think about. And this was a good opportunity to re-read the
code.)
I also still had references to the always-empty-string Prefix field in
the template.
Change-Id: I4fd9002d6d1bae2a08f4c53ec34651a7fe5db344
Reviewed-on: https://go-review.googlesource.com/c/build/+/208399
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/app/appengine/build.go b/app/appengine/build.go
index b4a4825..25da662 100644
--- a/app/appengine/build.go
+++ b/app/appengine/build.go
@@ -17,6 +17,7 @@
"golang.org/x/build/dashboard"
"golang.org/x/build/internal/loghash"
+ "google.golang.org/appengine"
"google.golang.org/appengine/datastore"
)
@@ -44,6 +45,32 @@
return datastore.NewKey(c, "Package", key, 0, nil)
}
+// filterDatastoreError returns err, unless it's just about datastore
+// not being able to load an entity with old legacy struct fields into
+// the Commit type that has since removed those fields.
+func filterDatastoreError(err error) error {
+ if em, ok := err.(*datastore.ErrFieldMismatch); ok {
+ switch em.FieldName {
+ case "NeedsBenchmarking", "TryPatch", "FailNotificationSent":
+ // Removed in CLs 208397 and 208324.
+ return nil
+ }
+ }
+ if me, ok := err.(appengine.MultiError); ok {
+ any := false
+ for i, err := range me {
+ me[i] = filterDatastoreError(err)
+ if me[i] != nil {
+ any = true
+ }
+ }
+ if !any {
+ return nil
+ }
+ }
+ return err
+}
+
// LastCommit returns the most recent Commit for this Package.
func (p *Package) LastCommit(c context.Context) (*Commit, error) {
var commits []*Commit
@@ -52,6 +79,7 @@
Order("-Time").
Limit(1).
GetAll(c, &commits)
+ err = filterDatastoreError(err)
if err != nil {
return nil, err
}
@@ -139,7 +167,9 @@
// AddResult adds the denormalized Result data to the Commit's ResultData field.
// It must be called from inside a datastore transaction.
func (com *Commit) AddResult(c context.Context, r *Result) error {
- if err := datastore.Get(c, com.Key(c), com); err != nil {
+ err := datastore.Get(c, com.Key(c), com)
+ err = filterDatastoreError(err)
+ if err != nil {
return fmt.Errorf("getting Commit: %v", err)
}
@@ -408,6 +438,7 @@
func (t *Tag) Commit(c context.Context) (*Commit, error) {
com := &Commit{Hash: t.Hash}
err := datastore.Get(c, com.Key(c), com)
+ err = filterDatastoreError(err)
return com, err
}
diff --git a/app/appengine/handler.go b/app/appengine/handler.go
index 58be380..c60335a 100644
--- a/app/appengine/handler.go
+++ b/app/appengine/handler.go
@@ -170,6 +170,7 @@
Filter("Hash =", com.ParentHash).
Ancestor(p.Key(c)).
Count(c)
+ err = filterDatastoreError(err)
if err != nil {
return fmt.Errorf("testing for parent Commit: %v", err)
}
@@ -374,6 +375,7 @@
Order("-Num").
Limit(commitsPerPage).
GetAll(c, &coms)
+ err = filterDatastoreError(err)
if err != nil {
return err
}
diff --git a/app/appengine/ui.go b/app/appengine/ui.go
index 2b3f769..725c8b1 100644
--- a/app/appengine/ui.go
+++ b/app/appengine/ui.go
@@ -114,6 +114,7 @@
if err == datastore.ErrNoSuchEntity {
continue
}
+ err = filterDatastoreError(err)
if err != nil {
logErr(w, r, err)
return
@@ -195,6 +196,7 @@
func listBranches(c context.Context) (branches []string) {
var commits []*Commit
_, err := datastore.NewQuery("Commit").Distinct().Project("Branch").GetAll(c, &commits)
+ err = filterDatastoreError(err)
if err != nil {
log.Errorf(c, "listBranches: %v", err)
return
@@ -318,6 +320,7 @@
var commits []*Commit
_, err := q.Limit(commitsPerPage).Offset(offset).
GetAll(c, &commits)
+ err = filterDatastoreError(err)
// If we're running locally and don't have data, return some test data.
// This lets people hack on the UI without setting up gitmirror & friends.
@@ -348,6 +351,7 @@
keys = append(keys, commit.Key(c))
}
err := datastore.GetMulti(c, keys, out)
+ err = filterDatastoreError(err)
return out, err
}
diff --git a/app/appengine/ui.html b/app/appengine/ui.html
index ca7ec4e..c19bbc0 100644
--- a/app/appengine/ui.html
+++ b/app/appengine/ui.html
@@ -125,7 +125,7 @@
{{else if .OK}}
<span class="ok">ok</span>
{{else}}
- <a href="{{$.Dashboard.Prefix}}/log/{{.LogHash}}" class="fail">fail</a>
+ <a href="/log/{{.LogHash}}" class="fail">fail</a>
{{end}}
{{else}}
@@ -225,7 +225,7 @@
{{else if .OK}}
<span class="ok">ok</span>
{{else}}
- <a href="{{$.Dashboard.Prefix}}/log/{{.LogHash}}" class="fail">fail</a>
+ <a href="/log/{{.LogHash}}" class="fail">fail</a>
{{end}}
{{else}}
