Skip to content

Commit

Permalink
Merge branch 'flutter:main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
ricardoamador authored Feb 21, 2024
2 parents ac9a943 + 673a991 commit eda8ebe
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 13 deletions.
27 changes: 21 additions & 6 deletions app_dart/lib/src/request_handlers/scheduler/batch_backfiller.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,21 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:cocoon_service/src/foundation/utils.dart';
import 'package:cocoon_service/cocoon_service.dart';
import 'package:cocoon_service/src/model/appengine/task.dart';
import 'package:cocoon_service/src/request_handling/body.dart';
import 'package:cocoon_service/src/model/firestore/task.dart' as firestore;
import 'package:cocoon_service/src/service/datastore.dart';
import 'package:cocoon_service/src/service/scheduler/policy.dart';
import 'package:gcloud/db.dart';
import 'package:github/github.dart';
import 'package:googleapis/firestore/v1.dart';
import 'package:meta/meta.dart';
import 'package:retry/retry.dart';

import '../../model/ci_yaml/ci_yaml.dart';
import '../../model/ci_yaml/target.dart';
import '../../request_handling/exceptions.dart';
import '../../request_handling/request_handler.dart';
import '../../service/config.dart';
import '../../service/logging.dart';
import '../../service/luci_build_service.dart';
import '../../service/scheduler.dart';

/// Cron request handler for scheduling targets when capacity becomes available.
///
Expand Down Expand Up @@ -103,6 +100,13 @@ class BatchBackfiller extends RequestHandler {
'Updated ${backfillTasks.length} tasks: ${backfillTasks.map((e) => e.name).toList()} when backfilling.',
);
});
// TODO(keyonghan): remove try catch logic after validated to work.
try {
await updateTaskDocuments(backfillTasks);
} catch (error) {
log.warning('Failed to update batch backfilled task documents in Firestore: $error');
}

// Schedule all builds asynchronously.
// Schedule after db updates to avoid duplicate scheduling when db update fails.
await _scheduleWithRetries(backfill);
Expand All @@ -111,6 +115,17 @@ class BatchBackfiller extends RequestHandler {
}
}

/// Updates task documents in Firestore.
Future<void> updateTaskDocuments(List<Task> tasks) async {
if (tasks.isEmpty) {
return;
}
final List<firestore.Task> taskDocuments = tasks.map((e) => taskToTaskDocument(e)).toList();
final List<Write> writes = documentsToWrites(taskDocuments, exists: true);
final FirestoreService firestoreService = await config.createFirestoreService();
await firestoreService.writeViaTransaction(writes);
}

/// Filters [config.backfillerTargetLimit] targets to backfill.
///
/// High priority targets will be guranteed to get back filled first. If more targets
Expand Down
22 changes: 21 additions & 1 deletion app_dart/lib/src/service/firestore.dart
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ List<Document> targetsToTaskDocuments(Commit commit, List<Target> targets) {
kTaskCreateTimestampField: Value(integerValue: commit.timestamp!.toString()),
kTaskEndTimestampField: Value(integerValue: kTaskDefaultTimestampValue.toString()),
kTaskBringupField: Value(booleanValue: target.value.bringup),
kTaskNameField: Value(stringValue: target.value.name.toString()),
kTaskNameField: Value(stringValue: target.value.name),
kTaskStartTimestampField: Value(integerValue: kTaskDefaultTimestampValue.toString()),
kTaskStatusField: Value(stringValue: Task.statusNew),
kTaskTestFlakyField: Value(booleanValue: false),
Expand All @@ -146,6 +146,26 @@ Document commitToCommitDocument(Commit commit) {
);
}

/// Generates task document based on datastore task data model.
firestore.Task taskToTaskDocument(Task task) {
final String commitSha = task.commitKey!.id!.split('/').last;
return firestore.Task.fromDocument(
taskDocument: Document(
name: '$kDatabase/documents/$kTaskCollectionId/${commitSha}_${task.name}_${task.attempts}',
fields: <String, Value>{
kTaskCreateTimestampField: Value(integerValue: task.createTimestamp.toString()),
kTaskEndTimestampField: Value(integerValue: task.endTimestamp.toString()),
kTaskBringupField: Value(booleanValue: task.isFlaky),
kTaskNameField: Value(stringValue: task.name),
kTaskStartTimestampField: Value(integerValue: task.startTimestamp.toString()),
kTaskStatusField: Value(stringValue: task.status),
kTaskTestFlakyField: Value(booleanValue: task.isTestFlaky),
kTaskCommitShaField: Value(stringValue: commitSha),
},
),
);
}

/// Creates a list of [Write] based on documents.
List<Write> documentsToWrites(List<Document> documents, {bool exists = false}) {
return documents
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import 'package:cocoon_service/cocoon_service.dart';
import 'package:cocoon_service/src/model/appengine/commit.dart';
import 'package:cocoon_service/src/model/appengine/task.dart';
import 'package:cocoon_service/src/model/firestore/task.dart' as firestore;
import 'package:cocoon_service/src/model/ci_yaml/target.dart';
import 'package:cocoon_service/src/model/luci/buildbucket.dart';
import 'package:googleapis/firestore/v1.dart';
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

Expand All @@ -33,11 +35,17 @@ void main() {
late FakeScheduler scheduler;
late MockGithubChecksUtil mockGithubChecksUtil;
late Config config;
late MockFirestoreService mockFirestoreService;

group('BatchBackfiller', () {
setUp(() async {
mockFirestoreService = MockFirestoreService();
db = FakeDatastoreDB()..addOnQuery<Commit>((Iterable<Commit> results) => commits);
config = FakeConfig(dbValue: db, backfillerTargetLimitValue: 2);
config = FakeConfig(
dbValue: db,
backfillerTargetLimitValue: 2,
firestoreService: mockFirestoreService,
);
pubsub = FakePubSub();
mockGithubChecksUtil = MockGithubChecksUtil();
when(
Expand All @@ -49,6 +57,13 @@ void main() {
output: anyNamed('output'),
),
).thenAnswer((_) async => generateCheckRun(1));
when(
mockFirestoreService.writeViaTransaction(
captureAny,
),
).thenAnswer((Invocation invocation) {
return Future<CommitResponse>.value(CommitResponse());
});
scheduler = FakeScheduler(
config: config,
ciYaml: batchPolicyConfig,
Expand Down Expand Up @@ -202,6 +217,13 @@ void main() {
await tester.get(handler);
expect(db.values.length, 1);
expect(task.status, Task.statusInProgress);

final List<dynamic> captured = verify(mockFirestoreService.writeViaTransaction(captureAny)).captured;
expect(captured.length, 1);
final List<Write> commitResponse = captured[0] as List<Write>;
expect(commitResponse.length, 1);
final firestore.Task taskDocuemnt = firestore.Task.fromDocument(taskDocument: commitResponse[0].update!);
expect(taskDocuemnt.status, firestore.Task.statusInProgress);
});

test('skip scheduling builds if datastore commit fails', () async {
Expand All @@ -218,7 +240,7 @@ void main() {
expect(pubsub.messages.length, 0);
});

test('backfills only column A when B does need backfill', () async {
test('backfills only column A when B does not need backfill', () async {
final List<Task> scheduleA = <Task>[
// Linux_android A
generateTask(1, name: 'Linux_android A', status: Task.statusSucceeded),
Expand Down
16 changes: 16 additions & 0 deletions app_dart/test/service/firestore_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'package:cocoon_service/src/model/appengine/commit.dart';
import 'package:cocoon_service/src/model/appengine/task.dart';
import 'package:cocoon_service/src/model/firestore/task.dart' as firestore;
import 'package:cocoon_service/src/model/ci_yaml/target.dart';
import 'package:cocoon_service/src/service/firestore.dart';

Expand Down Expand Up @@ -45,6 +46,21 @@ void main() {
expect(commitDocument.fields![kCommitShaField]!.stringValue, commit.sha);
});

test('creates task document correctly from task data model', () async {
final Task task = generateTask(1);
final String commitSha = task.commitKey!.id!.split('/').last;
final firestore.Task taskDocument = taskToTaskDocument(task);
expect(taskDocument.name, '$kDatabase/documents/$kTaskCollectionId/${commitSha}_${task.name}_${task.attempts}');
expect(taskDocument.createTimestamp, task.createTimestamp);
expect(taskDocument.endTimestamp, task.endTimestamp);
expect(taskDocument.bringup, task.isFlaky);
expect(taskDocument.taskName, task.name);
expect(taskDocument.startTimestamp, task.startTimestamp);
expect(taskDocument.status, task.status);
expect(taskDocument.testFlaky, task.isTestFlaky);
expect(taskDocument.commitSha, commitSha);
});

test('creates writes correctly from documents', () async {
final List<Document> documents = <Document>[
Document(name: 'd1', fields: <String, Value>{'key1': Value(stringValue: 'value1')}),
Expand Down
8 changes: 8 additions & 0 deletions packages/buildbucket-dart/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
# 1.0.9

- Pin versions due to https://github.com/flutter/flutter/issues/110272

# 1.0.8

- Update dependencies.

# 1.0.7

- Expose the google.protobuf dependency objects.
Expand Down
11 changes: 11 additions & 0 deletions packages/buildbucket-dart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ These protobufs are used to communicate with LUCI Buildbucket services from Dart
That will checkout protobuf, buildbucket and googleapis repositories. It will also compile the protos
and generate their correspondent Dart classes.

## Validating Changes

In order to validate the changes you have made before releasing a new version you can point your project
to the local directory for packages/buildbucket-dart by doing the following in your project's pubspec.yaml:

```dart
dependencies:
buildbucket-dart:
path: /your/path/to/packages/buildbucket-dart
```

## Feedback

File an issue in flutter/flutter with the word 'buildbucket-dart' clearly in the title and cc @godofredoc.
11 changes: 9 additions & 2 deletions packages/buildbucket-dart/lib/buildbucket_pb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ export 'src/generated/go.chromium.org/luci/buildbucket/proto/builds_service.pb.d
ScheduleBuildRequest_Swarming,
StartBuildRequest,
StartBuildResponse,
StartBuildTaskRequest,
StartBuildTaskResponse,
CancelBuildRequest,
CreateBuildRequest,
SynthesizeBuildRequest,
Expand Down Expand Up @@ -78,3 +76,12 @@ export 'src/generated/go.chromium.org/luci/buildbucket/proto/common.pb.dart'
export 'src/generated/go.chromium.org/luci/buildbucket/proto/common.pbenum.dart';
export 'src/generated/go.chromium.org/luci/buildbucket/proto/notification.pb.dart'
show NotificationConfig, BuildsV2PubSub, PubSubCallBack;

export 'src/generated/google/protobuf/struct.pb.dart' show Struct, Value, Value_Kind, NullValue, ListValue;
export 'src/generated/google/protobuf/any.pb.dart' show Any;
export 'src/generated/google/protobuf/duration.pb.dart' show Duration;
export 'src/generated/google/protobuf/empty.pb.dart' show Empty;
export 'src/generated/google/protobuf/timestamp.pb.dart' show Timestamp;
export 'src/generated/google/protobuf/wrappers.pb.dart'
show DoubleValue, FloatValue, Int32Value, Int64Value, BoolValue, BytesValue, UInt32Value, UInt64Value, StringValue;
export 'src/generated/google/protobuf/field_mask.pb.dart' show FieldMask;
4 changes: 2 additions & 2 deletions packages/buildbucket-dart/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: buildbucket
description: LUCI Buildbucket library. Protos and utilities to communicate with LUCI buildbucket service.
version: 1.0.6
version: 1.0.9
repository: https://github.com/flutter/cocoon/tree/main/packages/buildbucket-dart

environment:
Expand All @@ -14,4 +14,4 @@ dependencies:

dev_dependencies:
lints: 3.0.0
test: 1.24.8
test: 1.25.2

0 comments on commit eda8ebe

Please sign in to comment.