Skip to content

Refactor and split up DynamicConfig*, prepare for more flag groups. #4732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 2 commits into from
May 29, 2025
Merged

Refactor and split up DynamicConfig*, prepare for more flag groups. #4732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aedb803
Refactor and split up DynamicConfig, prepare for more flag groups.
matanlurey May 28, 2025
4ca6abe
Feedback.
matanlurey May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app_dart/bin/gae_server.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'package:cocoon_service/src/service/build_status_service.dart';
import 'package:cocoon_service/src/service/commit_service.dart';
import 'package:cocoon_service/src/service/content_aware_hash_service.dart';
import 'package:cocoon_service/src/service/firebase_jwt_validator.dart';
import 'package:cocoon_service/src/service/flags/dynamic_config_updater.dart';
import 'package:cocoon_service/src/service/get_files_changed.dart';
import 'package:cocoon_service/src/service/scheduler/ci_yaml_fetcher.dart';
import 'package:logging/logging.dart';
Expand Down Expand Up @@ -47,7 +48,7 @@ Future<void> main() async {
const GoogleAuthProvider(),
projectId: Config.flutterGcpProjectId,
),
dynamicConfig: dynamicConfig,
initialConfig: dynamicConfig,
);
// Start updating the config to loop forever. If this fails, it will log
// every ~1 minute.
Expand Down
183 changes: 3 additions & 180 deletions app_dart/lib/src/service/config.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// found in the LICENSE file.

import 'dart:convert';
import 'dart:math' show Random;
import 'dart:typed_data';

import 'package:cocoon_server/generate_github_jws.dart';
Expand All @@ -12,30 +11,20 @@ import 'package:cocoon_server/secret_manager.dart';
import 'package:github/github.dart' as gh;
import 'package:graphql/client.dart' hide JsonSerializable;
import 'package:http/http.dart' as http;
import 'package:json_annotation/json_annotation.dart';
import 'package:meta/meta.dart';
import 'package:retry/retry.dart';
import 'package:yaml/yaml.dart' show YamlList, YamlMap, loadYaml;

import '../../cocoon_service.dart';
import '../foundation/providers.dart' show Providers;
import '../foundation/typedefs.dart' show HttpClientProvider;
import 'flags/content_aware_hashing_flags.dart';
import 'flags/dynamic_config_updater.dart';
import 'github_service.dart';
import 'luci_build_service/cipd_version.dart';

part 'config.g.dart';

/// Name of the default git branch.
const String kDefaultBranchName = 'master';

interface class Config {
interface class Config extends DynamicallyUpdatedConfig {

@jtmcdole jtmcdole May 29, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

/// Creates and returns a [Config] instance.
Config(this._cache, this._secrets, {required DynamicConfig dynamicConfig})
: _dynamicConfig = dynamicConfig;

/// Access dynamically configured flags.
DynamicConfig get flags => _dynamicConfig;
Config(this._cache, this._secrets, {required super.initialConfig});

/// When present on a pull request, instructs Cocoon to submit it
/// automatically as soon as all the required checks pass.
Expand Down Expand Up @@ -71,8 +60,6 @@ interface class Config {
final CacheService _cache;
final SecretManager _secrets;

DynamicConfig _dynamicConfig;

/// List of Github presubmit supported repos.
///
/// This adds support for the `waiting for tree to go green label` to the repo.
Expand Down Expand Up @@ -497,167 +484,3 @@ interface class Config {
return GithubService(github);
}
}

/// Flags for the service that can be updated dynamically with out a restart.
///
/// Should be read from git/HEAD/app_dart/config.yaml and cached between
/// services.
@JsonSerializable(explicitToJson: true)
@immutable
final class DynamicConfig {
/// Upper limit of commit rows to be backfilled in API call.
///
/// This limits the number of commits to be checked to backfill. When bots
/// are idle, we hope to scan as many commit rows as possible.
@JsonKey(defaultValue: 50)
final int backfillerCommitLimit;

final ContentAwareHashingJson contentAwareHashing;

DynamicConfig({
required this.backfillerCommitLimit,
required this.contentAwareHashing,
});

/// Connect the generated [_$DynamicConfigFromJson] function to the `fromJson`
/// factory.
factory DynamicConfig.fromJson(Map<String, Object?>? json) =>
_$DynamicConfigFromJson(json ?? {});

/// Connect the generated [_$DynamicConfigToJson] function to the `toJson` method.
Map<String, dynamic> toJson() => _$DynamicConfigToJson(this);
}

extension YamlMapToMap on YamlMap {
Map<String, Object?> get asMap => <String, Object?>{
for (final MapEntry(:key, :value) in entries)
if (value is YamlMap)
'$key': value.asMap
else if (value is YamlList)
'$key': value.asList
else
'$key': value,
};
}

extension YamlListToList on YamlList {
List<Object?> get asList => <Object?>[
for (final value in nodes)
if (value is YamlMap)
value.asMap
else if (value is YamlList)
value.asList
else
value,
];
}

/// Responsibly polls for configuration changes to our service config.
///
/// This works by fetching the latest checked in "config.yaml".
class DynamicConfigUpdater {
DynamicConfigUpdater({
Duration delay = const Duration(minutes: 1),
@visibleForTesting Random? random,
@visibleForTesting
HttpClientProvider httpClientProvider = Providers.freshHttpClient,
@visibleForTesting
RetryOptions retryOptions = const RetryOptions(
maxAttempts: 3,
delayFactor: Duration(seconds: 3),
),
}) : _delay = delay,
_random = random ?? Random(),
_httpClientProvider = httpClientProvider,
_retryOptions = retryOptions;

final Duration _delay;
final Random _random;
final HttpClientProvider _httpClientProvider;
final RetryOptions _retryOptions;

/// Fetches and parses the `config.yaml` from HEAD `flutter/cocoon/app_dart/`.
Future<DynamicConfig> fetchDynamicConfig() async {
final file = await githubFileContent(
Config.cocoonSlug,
'app_dart/config.yaml',
ref: 'main',
httpClientProvider: _httpClientProvider,
retryOptions: _retryOptions,
);
final configYaml = loadYaml(file) as YamlMap;
return DynamicConfig.fromJson(configYaml.asMap);
}

UpdaterStatus _status = UpdaterStatus.stopped;

void stopUpdateLoop() {
if (_status != UpdaterStatus.running) return;
log.info('ConfigUpdater: Stopping config update loop...');
_status = UpdaterStatus.stopping;
}

void startUpdateLoop(Config config) async {
if (_status != UpdaterStatus.stopped) return;
_status = UpdaterStatus.running;

log.info('ConfigUpdater: Starting config update loop...');

// What we've decided:
// 1. Each instance will **start** with a valid DynamicConfig
// 2. Each instance will update their own config on an interval that can
// drift by as much as a minute.
// 3. If a fetch fails, we'll log an error, but keep using the last config
while (true) {
await Future<void>.delayed(
_delay + Duration(milliseconds: _random.nextInt(1000)),
);
if (_status != UpdaterStatus.running) {
log.info('ConfigUpdater: Stopped config update loop');
_status = UpdaterStatus.stopped;
return;
}
try {
final dynamicConfig = await fetchDynamicConfig();
final diffs = diffConfigChanges(
config._dynamicConfig.toJson(),
dynamicConfig.toJson(),
);
if (diffs.isNotEmpty) {
log.info('ConfigUpdater: ${diffs.join(',')}');
config._dynamicConfig = dynamicConfig;
}
} catch (e, s) {
log.error('ConfigUpdater: Unable to fetch DynamicConfig!', e, s);
}
}
}

/// Produce a simple diff of the changing flags.
List<String> diffConfigChanges(
Map<String, Object?> oldFlags,
Map<String, Object?> newFlags, {
List<String>? diffs,
String chain = 'flags',
}) {
diffs ??= <String>[];

for (final MapEntry(:key, :value) in oldFlags.entries) {
if (value is Map) {
diffConfigChanges(
value as Map<String, Object?>,
newFlags[key] as Map<String, Object?>,
diffs: diffs,
chain: '$chain.$key',
);
continue;
}
if (value != newFlags[key]) {
diffs.add('$chain.$key $value -> ${newFlags[key]}');
}
}
return diffs;
}
}

enum UpdaterStatus { stopped, running, stopping }
39 changes: 29 additions & 10 deletions app_dart/lib/src/service/flags/content_aware_hashing_flags.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,39 @@ import 'package:meta/meta.dart';

part 'content_aware_hashing_flags.g.dart';

/// Flags related to content-aware hashing.
@JsonSerializable()
@immutable
final class ContentAwareHashingJson {
ContentAwareHashingJson({required this.waitOnContentHash});
final class ContentAwareHashing {
/// Default configuration for [ContentAwareHashing] flags.
static const defaultInstance = ContentAwareHashing._(
waitOnContentHash: false,
);

/// Merge Groups should wait for the content hash before scheduling.
@JsonKey(defaultValue: false)
/// Whether merge groups should wait for the content hash before scheduling.
@JsonKey()
final bool waitOnContentHash;

/// Connect the generated [_$ContentAwareHashingJsonFromJson] function to the `fromJson`
/// factory.
factory ContentAwareHashingJson.fromJson(Map<String, Object?>? json) =>
_$ContentAwareHashingJsonFromJson(json ?? {});
const ContentAwareHashing._({
required this.waitOnContentHash, //
});

/// Connect the generated [_$ContentAwareHashingJsonToJson] function to the `toJson` method.
Map<String, dynamic> toJson() => _$ContentAwareHashingJsonToJson(this);
/// Creates [ContentAwareHashing] flags from the provided fields.
///
/// Any omitted fields default to the values in [defaultInstance].
factory ContentAwareHashing({bool? waitOnContentHash}) {
return ContentAwareHashing._(
waitOnContentHash: waitOnContentHash ?? defaultInstance.waitOnContentHash,
);
}

/// Creates [ContentAwareHashing] flags from a [json] object.
///
/// Any omitted fields default to the values in [defaultInstance].
factory ContentAwareHashing.fromJson(Map<String, Object?>? json) {
return _$ContentAwareHashingFromJson(json ?? {});
}

/// The inverse operation of [ContentAwareHashing.fromJson].
Map<String, Object?> toJson() => _$ContentAwareHashingToJson(this);
}
11 changes: 4 additions & 7 deletions app_dart/lib/src/service/flags/content_aware_hashing_flags.g.dart
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading