feat(dev-infra): provide github API instance to lazy merge configuration (#38223)

The merge script currently accepts a configuration function that will
be invoked _only_ when the `ng-dev merge` command is executed. This
has been done that way because the merge tooling usually relies on
external requests to Git or NPM for constructing the branch configurations.

We do not want to perform these slow external queries on any `ng-dev` command
though, so this became a lazily invoked function.

This commit adds support for these configuration functions to run
asynchronously (by returning a Promise that will be awaited), so that
requests could also be made to the Github API. This is benefical as it
could avoid dependence on the local Git state and the HTTP requests
are more powerful/faster.

Additionally, in order to be able to perform Github API requests
with an authenticated instance, the merge tool will pass through
a `GithubClient` instance that uses the specified `--github-token`
(or from the environment). This ensures that all API requests use
the same `GithubClient` instance and can be authenticated (mitigating
potential rate limits).

PR Close #38223
This commit is contained in:
Paul Gschwendtner 2020-07-24 17:59:12 +02:00 committed by Andrew Kushnir
parent 585ef2bdd7
commit 576e329f33
5 changed files with 56 additions and 56 deletions

View File

@ -6,10 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/
import {getConfig, GitClientConfig, NgDevConfig} from '../../utils/config';
import {GitClientConfig, NgDevConfig} from '../../utils/config';
import {GithubClient} from '../../utils/git/github';
import {GithubApiMergeStrategyConfig} from './strategies/api-merge';
/** Describes possible values that can be returned for `branches` of a target label. */
export type TargetLabelBranchResult = string[]|Promise<string[]>;
/**
* Possible merge methods supported by the Github API.
* https://developer.github.com/v3/pulls/#merge-a-pull-request-merge-button.
@ -28,7 +32,7 @@ export interface TargetLabel {
* Can also be wrapped in a function that accepts the target branch specified in the
* Github Web UI. This is useful for supporting labels like `target: development-branch`.
*/
branches: string[]|((githubTargetBranch: string) => string[]);
branches: TargetLabelBranchResult|((githubTargetBranch: string) => TargetLabelBranchResult);
}
/**
@ -72,12 +76,13 @@ export interface MergeConfig {
* on branch name computations. We don't want to run these immediately whenever
* the dev-infra configuration is loaded as that could slow-down other commands.
*/
export type DevInfraMergeConfig = NgDevConfig<{'merge': () => MergeConfig}>;
export type DevInfraMergeConfig =
NgDevConfig<{'merge': (api: GithubClient) => MergeConfig | Promise<MergeConfig>}>;
/** Loads and validates the merge configuration. */
export function loadAndValidateConfig(): {config?: MergeConfigWithRemote, errors?: string[]} {
const config: Partial<DevInfraMergeConfig> = getConfig();
export async function loadAndValidateConfig(
config: Partial<DevInfraMergeConfig>,
api: GithubClient): Promise<{config?: MergeConfig, errors?: string[]}> {
if (config.merge === undefined) {
return {errors: ['No merge configuration found. Set the `merge` configuration.']};
}
@ -86,22 +91,14 @@ export function loadAndValidateConfig(): {config?: MergeConfigWithRemote, errors
return {errors: ['Expected merge configuration to be defined lazily through a function.']};
}
const mergeConfig = config.merge();
const mergeConfig = await config.merge(api);
const errors = validateMergeConfig(mergeConfig);
if (errors.length) {
return {errors};
}
if (mergeConfig.remote) {
mergeConfig.remote = {...config.github, ...mergeConfig.remote};
} else {
mergeConfig.remote = config.github;
}
// We always set the `remote` option, so we can safely cast the
// config to `MergeConfigWithRemote`.
return {config: mergeConfig as MergeConfigWithRemote};
return {config: mergeConfig};
}
/** Validates the specified configuration. Returns a list of failure messages. */

View File

@ -7,11 +7,12 @@
*/
import {getRepoBaseDir} from '../../utils/config';
import {getConfig, getRepoBaseDir} from '../../utils/config';
import {error, green, info, promptConfirm, red, yellow} from '../../utils/console';
import {GithubApiRequestError} from '../../utils/git';
import {GitClient} from '../../utils/git';
import {GithubApiRequestError} from '../../utils/git/github';
import {loadAndValidateConfig, MergeConfigWithRemote} from './config';
import {loadAndValidateConfig, MergeConfig, MergeConfigWithRemote} from './config';
import {MergeResult, MergeStatus, PullRequestMergeTask} from './task';
/** URL to the Github page where personal access tokens can be generated. */
@ -34,19 +35,7 @@ export const GITHUB_TOKEN_GENERATE_URL = `https://github.com/settings/tokens`;
export async function mergePullRequest(
prNumber: number, githubToken: string, projectRoot: string = getRepoBaseDir(),
config?: MergeConfigWithRemote) {
// If no explicit configuration has been specified, we load and validate
// the configuration from the shared dev-infra configuration.
if (config === undefined) {
const {config: _config, errors} = loadAndValidateConfig();
if (errors) {
error(red('Invalid configuration:'));
errors.forEach(desc => error(yellow(` - ${desc}`)));
process.exit(1);
}
config = _config!;
}
const api = new PullRequestMergeTask(projectRoot, config, githubToken);
const api = await createPullRequestMergeTask(githubToken, projectRoot, config);
// Perform the merge. Force mode can be activated through a command line flag.
// Alternatively, if the merge fails with non-fatal failures, the script
@ -132,3 +121,33 @@ export async function mergePullRequest(
}
}
}
/**
* Creates the pull request merge task from the given Github token, project root
* and optional explicit configuration. An explicit configuration can be specified
* when the merge script is used outside of a `ng-dev` configured repository.
*/
async function createPullRequestMergeTask(
githubToken: string, projectRoot: string, explicitConfig?: MergeConfigWithRemote) {
if (explicitConfig !== undefined) {
const git = new GitClient(githubToken, {github: explicitConfig.remote}, projectRoot);
return new PullRequestMergeTask(explicitConfig, git);
}
const devInfraConfig = getConfig();
const git = new GitClient(githubToken, devInfraConfig, projectRoot);
const {config, errors} = await loadAndValidateConfig(devInfraConfig, git.github);
if (errors) {
error(red('Invalid merge configuration:'));
errors.forEach(desc => error(yellow(` - ${desc}`)));
process.exit(1);
}
// Set the remote so that the merge tool has access to information about
// the remote it intends to merge to.
config!.remote = devInfraConfig.github;
// We can cast this to a merge config with remote because we always set the
// remote above.
return new PullRequestMergeTask(config! as MergeConfigWithRemote, git);
}

View File

@ -9,7 +9,7 @@
import {promptConfirm} from '../../utils/console';
import {GitClient, GitCommandError} from '../../utils/git';
import {MergeConfigWithRemote} from './config';
import {MergeConfig, MergeConfigWithRemote} from './config';
import {PullRequestFailure} from './failures';
import {getCaretakerNotePromptMessage} from './messages';
import {isPullRequest, loadAndValidatePullRequest,} from './pull-request';
@ -40,12 +40,7 @@ export interface MergeResult {
* labels that have been resolved through the merge script configuration.
*/
export class PullRequestMergeTask {
/** Git client that can be used to execute Git commands. */
git = new GitClient(this._githubToken, {github: this.config.remote});
constructor(
public projectRoot: string, public config: MergeConfigWithRemote,
private _githubToken: string) {}
constructor(public config: MergeConfigWithRemote, public git: GitClient) {}
/**
* Merges the given pull request and pushes it upstream.

View File

@ -6,13 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/
/****************************************************************************
****************************************************************************
** DO NOT IMPORT THE GithubClient DIRECTLY, INSTEAD IMPORT GitClient from **
** ./index.ts and access the GithubClient via the `.github` member. **
****************************************************************************
****************************************************************************/
import {graphql} from '@octokit/graphql';
import * as Octokit from '@octokit/rest';
import {RequestParameters} from '@octokit/types';
@ -28,10 +21,10 @@ export class GithubApiRequestError extends Error {
/**
* A Github client for interacting with the Github APIs.
*
* Additionally, provides convienience methods for actions which require multiple requests, or
* Additionally, provides convenience methods for actions which require multiple requests, or
* would provide value from memoized style responses.
**/
export class _GithubClient extends Octokit {
export class GithubClient extends Octokit {
/** The Github GraphQL (v4) API. */
graqhql: GithubGraphqlClient;

View File

@ -11,10 +11,7 @@ import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process';
import {getConfig, getRepoBaseDir, NgDevConfig} from '../config';
import {info, yellow} from '../console';
import {_GithubClient} from './_github';
// Re-export GithubApiRequestError
export {GithubApiRequestError} from './_github';
import {GithubClient} from './github';
/** Github response type extended to include the `x-oauth-scopes` headers presence. */
type RateLimitResponseWithOAuthScopeHeader = Octokit.Response<Octokit.RateLimitGetResponse>&{
@ -54,10 +51,8 @@ export class GitClient {
`https://${this._githubToken}@github.com/${this.remoteConfig.owner}/${
this.remoteConfig.name}.git`;
/** Instance of the authenticated Github octokit API. */
github = new _GithubClient(this._githubToken);
github = new GithubClient(this._githubToken);
/** The file path of project's root directory. */
private _projectRoot = getRepoBaseDir();
/** The OAuth scopes available for the provided Github token. */
private _oauthScopes: Promise<string[]>|null = null;
/**
@ -67,7 +62,8 @@ export class GitClient {
private _githubTokenRegex: RegExp|null = null;
constructor(
private _githubToken?: string, private _config: Pick<NgDevConfig, 'github'> = getConfig()) {
private _githubToken?: string, private _config: Pick<NgDevConfig, 'github'> = getConfig(),
private _projectRoot = getRepoBaseDir()) {
// If a token has been specified (and is not empty), pass it to the Octokit API and
// also create a regular expression that can be used for sanitizing Git command output
// so that it does not print the token accidentally.