First import #1

yjbanov · 2016-07-13T20:12:50Z

This is a basic app made of a Go App Engine server app and an Angular 2 Dart client app to get us going. It can:

Fetch commit data from GitHub and store it in datastore
Generate a checklist of build tasks
Show a build status table
Update build status from Travis
Provide a simple in-browser CLI for some management tasks (creating/authorizing agents)
Provide a basic JSON REST API for both the Angular app and build agents

devoncarew · 2016-07-13T21:21:48Z

app/lib/model.dart

+
+  AuthorInfo([Map<String, dynamic> props]) : super(_serializer, props);
+
+	String get login => this['Login'];


indent looks wonky here

For some reason Atom inserted tabs instead of spaces all over the place in this file. Fixed.

devoncarew · 2016-07-13T21:53:48Z

lgtm for my part; thanks for the walkthrough yesterday

skybrian · 2016-07-13T22:00:56Z

commands/refresh_travis_status.go

+					} else {
+						task.Status = "Failed"
+					}
+					cocoon.PutTask(taskEntity.Key, task)


I don't think this transaction does what you think. Since you are only doing a blind "put" inside the transaction, the datastore doesn't see any constraint on the order of the puts and they could be written in any order. Data can be lost.

If you want to safely update one field inside an entity without accidentally modifying any of the entity's other fields, the transaction should do a read-modify-write on the entity, entirely inside the transaction. (That is, doing a Get and a Put.)

(I'm also not sure why a bunch of unrelated tasks need to be updated within a single transaction, since presumably the Travis tasks can complete independently, in any order.)

You are right, there's no need for a transaction. Travis build status generally flips once and stays that way. It doesn't matter if we are trying to flip it from "New" to "Succeeded" concurrently. The end result is the same. We might support build reruns in the future, but that will be a separate feature and still very unlikely to cause conflicting concurrent status updates.

skybrian · 2016-07-14T22:11:08Z

db/db.go

+	return agent, authToken, nil
+}
+
+// CommitInfo contain information about a GitHub commit.


Just as an organization thing, maybe move all the structs corresponding to datastore entities (or parts of them) to schema.go?

skybrian · 2016-07-14T22:54:44Z

db/schema.go

+// independently. Different tasks belonging to the same Checklist can run in
+// parallel.
+type Task struct {
+	ChecklistKey *datastore.Key


Before going live you might want to consider if you should put the datastore entities into a hierarchy. Maybe something like:
Commit => Checklist => Task

The main advantage is that ancestor queries (under a single root) are atomic, versus other queries which are only eventually consistent. Also, you can do a transaction under a single root without making it a global transaction.

A disadvantage, though, is that each child entity's key is a suffix of the key of its parent. (So keys are longer and this makes URL's longer.) Also I think there's a limit of about 1 transaction/second under each root.

For more see:
https://cloud.google.com/appengine/docs/go/datastore/entities#Go_Ancestor_paths

Ancestor filter on this page:
https://cloud.google.com/appengine/docs/go/datastore/queries

There's no Commit records in the database, but Checklist => Task do form a parent-child relationship. If you look at PutTask, the key for Task is created as NewIncompleteKey(c.Ctx, "Task", task.ChecklistKey). While this field is a little redundant, I'd like to keep it because it makes it easy to marshal tasks to clients over JSON RPC. If I send just the encoded task key, the client would have to somehow parse the checklist key out of the task key.

...That said, I should use Ancestor(checklistKey) rather than Filter("ChecklistKey =", checklistKey). Will fix.

skybrian · 2016-07-14T22:55:05Z

LGTM

…ice class (#3026) We are getting alert for 500 errors from the /api/github/webhook-branch-subscription endpoint when attempting to insert a commit for a release branch. The issue is with the timestamp presumably being null when we attempt to insert it into the database since it not a field in the return RepositoryCommit in the github lib. ``` "Failed to process Instance of 'PushMessage'. (500) Invalid argument(s): Error while encoding entity (Bad state: Property validation failed for property timestamp while trying to serialize entity of kind Commit. , #0 _ModelDescription._encodeProperty (package:gcloud/src/db/model_db_impl.dart:449:7) #1 _ModelDescription.encodeModel.<anonymous closure> (package:gcloud/src/db/model_db_impl.dart:429:7) #2 _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:625:13) #3 _ModelDescription.encodeModel (package:gcloud/src/db/model_db_impl.dart:428:34) #4 ModelDBImpl.toDatastoreEntity (package:gcloud/src/db/model_db_impl.dart:121:31) #5 _commitHelper (package:gcloud/src/db/db.dart:434:38) #6 Transaction.commit (package:gcloud/src/db/db.dart:118:14) #7 DatastoreService.insert.<anonymous closure>.<anonymous closure> (package:cocoon_service/src/service/datastore.dart:254:31) #8 DatastoreDB.withTransaction.<anonymous closure> (package:gcloud/src/db/db.dart:324:32) #9 _rootRunUnary (dart:async/zone.dart:1407:47) <asynchronous suspension> #10 DatastoreService.insert.<anonymous closure> (package:cocoon_service/src/service/datastore.dart:252:11) <asynchronous suspension> #11 RetryOptions.retry (package:retry/retry.dart:131:16) <asynchronous suspension> #12 DatastoreService.insert (package:cocoon_service/src/service/datastore.dart:250:7) <asynchronous suspension> #13 CommitService.handleCreateGithubRequest (package:cocoon_service/src/service/commit_service.dart:42:7) <asynchronous suspension> #14 GithubBranchWebhookSubscription.post (package:cocoon_service/src/request_handlers/github/branch_subscription.dart:56:7) <asynchronous suspension> #15 RequestHandler.service.<anonymous closure> (package:cocoon_service/src/request_handling/request_handler.dart:53:24) <asynchronous suspension> #16 SubscriptionHandler.service (package:cocoon_service/src/request_handling/subscription_handler.dart:138:5) <asynchronous suspension> #17 main.<anonymous closure>.<anonymous closure> (file:///app/bin/server.dart:303:11) <asynchronous suspension> ``` The data returned by the call in the Commit Service looks like this: ``` { "sha": "6fd42536b7697eb4bd2a698b19308e0aacac70c7", "node_id": "C_kwDOAeUeuNoAKDZmZDQyNTM2Yjc2OTdlYjRiZDJhNjk4YjE5MzA4ZTBhYWNhYzcwYzc", "commit": { "author": { "name": "Xilai Zhang", "email": "xilaizhang@google.com", "date": "2023-08-30T02:59:02Z" }, "committer": { "name": "GitHub", "email": "noreply@github.com", "date": "2023-08-30T02:59:02Z" }, "message": "[flutter roll] Revert \"Fix `Chip.shape`'s side is not used when provided in Material 3\" (#133615)\n\nReverts flutter/flutter#132941\r\ncontext: b/298110031\r\n\r\nThe rounded rectangle borders don't appear in some of the internal\r\ngolden image tests.", "tree": { "sha": "e5efe7f39155d9b8fc40ad0a59c72f6a36f8b66d", "url": "https://api.github.com/repos/flutter/flutter/git/trees/e5efe7f39155d9b8fc40ad0a59c72f6a36f8b66d" }, ``` The author field in this case has no createdAt timestamp. *List which issues are fixed by this PR. You must list at least one issue.* Fixes flutter/flutter#133707 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

See flutter/flutter#152194 where I got befuddling error messages: ```txt Unhandled exception: Null check operator used on a null value #0 DeviceLabTestOwner.getTestOwnership (package:cocoon_service/src/request_handlers/test_ownership.dart:60:59) #1 getTestOwnership (package:cocoon_service/src/request_handlers/flaky_handler_utils.dart:301:20) #2 validateOwnership (package:cocoon_service/src/foundation/utils.dart:225:27) #3 remoteCheck (file:///b/s/w/ir/x/w/cocoon/app_dart/bin/validate_task_ownership.dart:31:40) <asynchronous suspension> #4 main (file:///b/s/w/ir/x/w/cocoon/app_dart/bin/validate_task_ownership.dart:63:23) <asynchronous suspension> ```

yjbanov added 30 commits July 13, 2016 13:05

Go App Engine app, ng2dart app

e7d0430

store tasks; better server errors

be0272c

add license and other boilerplate; update readme

cd445bd

api for fetching checklist statuses

6917e65

structured client-side API; display real data

30b1fd7

fix config files

5e9efe1

limit scope of uploaded files; fix a go vet warning

e24e9d0

build script; source maps

1a73294

deploy dart sources; fix indexes; fix crash when no data

f911259

encode dates as millis

5d62668

more robust dev_server

47872e8

agent capabilities; check out task

b0cebbc

/api/update-task-status

c5d1f9a

record time of task check out

b375334

Add oauth2 support

1aba993

verify OAuth2 tokens

c18ba70

building instructions; design summary

1c8adef

add Agent and agent sign in flow

a442830

create/auth agent CLI

02268cf

"check out" => "reserve"; refresh from CLI

8344eb2

fix and cli reserve-agent

5403a7b

clean up an import

6216cd4

authenticate age cron requests

f5eb5bc

switch to GAE users API

ec60f01

stages; some UI work

6756856

sort stages; more beauty

fa4e3a8

Put GitHub refresh on cron; fix JSON encoding

66bfb12

fix test

cba824f

Remove traces of oauth; log missing capabilities

26f4bdf

cron job to refresh Travis status

0576a71

devoncarew reviewed Jul 13, 2016
View reviewed changes

skybrian reviewed Jul 13, 2016
View reviewed changes

yjbanov added 3 commits July 13, 2016 18:25

cli-as-methods; readability fixes

00d3b4b

use current GAE Go API; fix header sorting on the UI

36408d3

move CurrentAgent into the Context

4bfba1e

skybrian reviewed Jul 14, 2016
View reviewed changes

yjbanov added 2 commits July 14, 2016 15:13

remove RunInTransaction non-helper; typed task status

89b0bb3

move DB schema into its own file

09e8d45

skybrian reviewed Jul 14, 2016
View reviewed changes

use ancestor query for tasks

b8a580f

yjbanov merged commit 5ede273 into flutter:master Jul 14, 2016

jonahwilliams mentioned this pull request Oct 7, 2019

Tree Build Status changes the AppBar color #452

Merged

yusufm mentioned this pull request Nov 9, 2020

Revert "Specify pubsub topic use when rescheduling prod builds." #992

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

First import #1

First import #1

yjbanov commented Jul 13, 2016 •

edited

Loading

Uh oh!

devoncarew Jul 13, 2016

Uh oh!

yjbanov Jul 13, 2016 •

edited

Loading

Uh oh!

devoncarew commented Jul 13, 2016

Uh oh!

skybrian Jul 13, 2016

Uh oh!

yjbanov Jul 14, 2016

Uh oh!

skybrian Jul 14, 2016

Uh oh!

yjbanov Jul 14, 2016

Uh oh!

skybrian Jul 14, 2016 •

edited

Loading

Uh oh!

yjbanov Jul 14, 2016

Uh oh!

yjbanov Jul 14, 2016

Uh oh!

skybrian commented Jul 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		AuthorInfo([Map<String, dynamic> props]) : super(_serializer, props);

		String get login => this['Login'];

First import #1

First import #1

Conversation

yjbanov commented Jul 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devoncarew Jul 13, 2016

Choose a reason for hiding this comment

Uh oh!

yjbanov Jul 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devoncarew commented Jul 13, 2016

Uh oh!

skybrian Jul 13, 2016

Choose a reason for hiding this comment

Uh oh!

yjbanov Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

skybrian Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

yjbanov Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

skybrian Jul 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yjbanov Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

yjbanov Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

skybrian commented Jul 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yjbanov commented Jul 13, 2016 •

edited

Loading

yjbanov Jul 13, 2016 •

edited

Loading

skybrian Jul 14, 2016 •

edited

Loading