Skip to content
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.

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

refactor(dev-infra): migrate github api in GitClient to rely on GithubClient #37593

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions dev-infra/pr/merge/pull-request.ts
Expand Up @@ -67,7 +67,7 @@ export async function loadAndValidatePullRequest(
}

const {data: {state}} =
await git.api.repos.getCombinedStatusForRef({...git.remoteParams, ref: prData.head.sha});
await git.github.repos.getCombinedStatusForRef({...git.remoteParams, ref: prData.head.sha});

if (state === 'failure' && !ignoreNonFatalFailures) {
return PullRequestFailure.failingCiJobs();
Expand Down Expand Up @@ -102,7 +102,7 @@ export async function loadAndValidatePullRequest(
async function fetchPullRequestFromGithub(
git: GitClient, prNumber: number): Promise<Octokit.PullsGetResponse|null> {
try {
const result = await git.api.pulls.get({...git.remoteParams, pull_number: prNumber});
const result = await git.github.pulls.get({...git.remoteParams, pull_number: prNumber});
return result.data;
} catch (e) {
// If the pull request could not be found, we want to return `null` so
Expand Down
6 changes: 3 additions & 3 deletions dev-infra/pr/merge/strategies/api-merge.ts
Expand Up @@ -94,7 +94,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {

try {
// Merge the pull request using the Github API into the selected base branch.
const result = await this.git.api.pulls.merge(mergeOptions);
const result = await this.git.github.pulls.merge(mergeOptions);

mergeStatusCode = result.status;
targetSha = result.data.sha;
Expand Down Expand Up @@ -189,9 +189,9 @@ export class GithubApiMergeStrategy extends MergeStrategy {

/** Gets all commit messages of commits in the pull request. */
private async _getPullRequestCommitMessages({prNumber}: PullRequest) {
const request = this.git.api.pulls.listCommits.endpoint.merge(
const request = this.git.github.pulls.listCommits.endpoint.merge(
{...this.git.remoteParams, pull_number: prNumber});
const allCommits: PullsListCommitsResponse = await this.git.api.paginate(request);
const allCommits: PullsListCommitsResponse = await this.git.github.paginate(request);
return allCommits.map(({commit}) => commit.message);
}

Expand Down
1 change: 1 addition & 0 deletions dev-infra/tmpl-package.json
Expand Up @@ -11,6 +11,7 @@
"dependencies": {
"@angular/benchpress": "0.2.0",
"@octokit/graphql": "<from-root>",
"@octokit/types": "<from-root>",
"brotli": "<from-root>",
"chalk": "<from-root>",
"cli-progress": "<from-root>",
Expand Down
6 changes: 5 additions & 1 deletion dev-infra/utils/BUILD.bazel
Expand Up @@ -2,12 +2,16 @@ load("@npm_bazel_typescript//:index.bzl", "ts_library")

ts_library(
name = "utils",
srcs = glob(["*.ts"]),
srcs = glob([
"*.ts",
"git/*.ts",
]),
module_name = "@angular/dev-infra-private/utils",
visibility = ["//dev-infra:__subpackages__"],
deps = [
"@npm//@octokit/graphql",
"@npm//@octokit/rest",
"@npm//@octokit/types",
"@npm//@types/inquirer",
"@npm//@types/node",
"@npm//@types/shelljs",
Expand Down
100 changes: 100 additions & 0 deletions dev-infra/utils/git/_github.ts
@@ -0,0 +1,100 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* 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';
import {query, types} from 'typed-graphqlify';

/** Error for failed Github API requests. */
export class GithubApiRequestError extends Error {
constructor(public status: number, message: string) {
super(message);
}
}

/**
* A Github client for interacting with the Github APIs.
*
* Additionally, provides convienience methods for actions which require multiple requests, or
* would provide value from memoized style responses.
**/
export class _GithubClient extends Octokit {
/** The Github GraphQL (v4) API. */
graqhql: GithubGraphqlClient;

/** The current user based on checking against the Github API. */
private _currentUser: string|null = null;

constructor(token?: string) {
// Pass in authentication token to base Octokit class.
super({auth: token});

this.hook.error('request', error => {
// Wrap API errors in a known error class. This allows us to
// expect Github API errors better and in a non-ambiguous way.
throw new GithubApiRequestError(error.status, error.message);
});

// Create authenticated graphql client.
this.graqhql = new GithubGraphqlClient(token);
}

/** Retrieve the login of the current user from Github. */
async getCurrentUser() {
// If the current user has already been retrieved return the current user value again.
if (this._currentUser !== null) {
return this._currentUser;
}
const result = await this.graqhql.query({
viewer: {
login: types.string,
}
});
return this._currentUser = result.viewer.login;
}
}

/**
* An object representation of a GraphQL Query to be used as a response type and to generate
* a GraphQL query string.
*/
type GraphQLQueryObject = Parameters<typeof query>[1];

/**
* A client for interacting with Github's GraphQL API.
*
* This class is intentionally not exported as it should always be access/used via a
* _GithubClient instance.
*/
class GithubGraphqlClient {
/** The Github GraphQL (v4) API. */
private graqhql = graphql;

constructor(token?: string) {
// Set the default headers to include authorization with the provided token for all
// graphQL calls.
if (token) {
this.graqhql.defaults({headers: {authorization: `token ${token}`}});
}
}


/** Perform a query using Github's GraphQL API. */
async query<T extends GraphQLQueryObject>(queryObject: T, params: RequestParameters = {}) {
const queryString = query(queryObject);
return (await this.graqhql(queryString, params)) as T;
}
}
29 changes: 8 additions & 21 deletions dev-infra/utils/git.ts → dev-infra/utils/git/index.ts
Expand Up @@ -9,22 +9,18 @@
import * as Octokit from '@octokit/rest';
import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process';

import {getConfig, getRepoBaseDir, NgDevConfig} from './config';
import {info, yellow} from './console';
import {getConfig, getRepoBaseDir, NgDevConfig} from '../config';
import {info, yellow} from '../console';
import {_GithubClient} from './_github';

// Re-export GithubApiRequestError
export {GithubApiRequestError} from './_github';

/** Github response type extended to include the `x-oauth-scopes` headers presence. */
type RateLimitResponseWithOAuthScopeHeader = Octokit.Response<Octokit.RateLimitGetResponse>&{
headers: {'x-oauth-scopes': string};
};

/** Error for failed Github API requests. */
export class GithubApiRequestError extends Error {
constructor(public status: number, message: string) {
super(message);
}
}

/** Error for failed Git commands. */
export class GitCommandError extends Error {
constructor(client: GitClient, public args: string[]) {
Expand Down Expand Up @@ -55,7 +51,7 @@ export class GitClient {
`https://${this._githubToken}@github.com/${this.remoteConfig.owner}/${
this.remoteConfig.name}.git`;
/** Instance of the authenticated Github octokit API. */
api: Octokit;
github = new _GithubClient(this._githubToken);

/** The file path of project's root directory. */
private _projectRoot = getRepoBaseDir();
Expand All @@ -75,13 +71,6 @@ export class GitClient {
if (_githubToken != null) {
this._githubTokenRegex = new RegExp(_githubToken, 'g');
}

this.api = new Octokit({auth: _githubToken});
this.api.hook.error('request', error => {
// Wrap API errors in a known error class. This allows us to
// expect Github API errors better and in a non-ambiguous way.
throw new GithubApiRequestError(error.status, error.message);
});
}

/** Executes the given git command. Throws if the command fails. */
Expand Down Expand Up @@ -186,10 +175,8 @@ export class GitClient {
return {error};
}


/**
* Retrieves the OAuth scopes for the loaded Github token, returning the already
* retrieved list of OAuth scopes if available.
* Retrieve the OAuth scopes for the loaded Github token.
**/
private async getAuthScopesForToken() {
// If the OAuth scopes have already been loaded, return the Promise containing them.
Expand All @@ -198,7 +185,7 @@ export class GitClient {
}
// OAuth scopes are loaded via the /rate_limit endpoint to prevent
// usage of a request against that rate_limit for this lookup.
return this._oauthScopes = this.api.rateLimit.get().then(_response => {
return this._oauthScopes = this.github.rateLimit.get().then(_response => {
const response = _response as RateLimitResponseWithOAuthScopeHeader;
const scopes: string = response.headers['x-oauth-scopes'] || '';
return scopes.split(',').map(scope => scope.trim());
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -62,6 +62,7 @@
"@bazel/typescript": "1.7.0",
"@microsoft/api-extractor": "7.7.11",
"@octokit/rest": "16.28.7",
"@octokit/types": "^5.0.1",
"@schematics/angular": "10.0.0-rc.2",
"@types/angular": "^1.6.47",
"@types/babel__core": "^7.1.6",
Expand Down
7 changes: 7 additions & 0 deletions yarn.lock
Expand Up @@ -1955,6 +1955,13 @@
dependencies:
"@types/node" ">= 8"

"@octokit/types@^5.0.1":
version "5.0.1"
resolved "https://registry.yarnpkg.com/@octokit/types/-/types-5.0.1.tgz#5459e9a5e9df8565dcc62c17a34491904d71971e"
integrity sha512-GorvORVwp244fGKEt3cgt/P+M0MGy4xEDbckw+K5ojEezxyMDgCaYPKVct+/eWQfZXOT7uq0xRpmrl/+hliabA==
dependencies:
"@types/node" ">= 8"

"@protobufjs/aspromise@^1.1.1", "@protobufjs/aspromise@^1.1.2":
version "1.1.2"
resolved "https://registry.yarnpkg.com/@protobufjs/aspromise/-/aspromise-1.1.2.tgz#9b8b0cc663d669a7d8f6f5d0893a14d348f30fbf"
Expand Down