Skip to content

Commit

Permalink
[kbnClient] Retry uiSettings.replace() calls up to 5 times (#52601)
Browse files Browse the repository at this point in the history
* [kbn/dev-utils] target ES2019 to transpile ??

* Retry uiSettings.replace() calls up to 5 times

* share logic for selecting junit report name to ensure they are unique

* convert to junit report path helper
  • Loading branch information
Spencer authored Dec 11, 2019
1 parent a25bf49 commit ab1fe3f
Show file tree
Hide file tree
Showing 10 changed files with 99 additions and 78 deletions.
70 changes: 36 additions & 34 deletions packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ export interface ReqOptions {
query?: Record<string, any>;
method: 'GET' | 'POST' | 'PUT' | 'DELETE';
body?: any;
attempt?: number;
maxAttempts?: number;
retries?: number;
}

const delay = (ms: number) =>
Expand All @@ -87,44 +86,47 @@ export class KbnClientRequester {
async request<T>(options: ReqOptions): Promise<T> {
const url = Url.resolve(this.pickUrl(), options.path);
const description = options.description || `${options.method} ${url}`;
const attempt = options.attempt === undefined ? 1 : options.attempt;
const maxAttempts =
options.maxAttempts === undefined ? DEFAULT_MAX_ATTEMPTS : options.maxAttempts;

try {
const response = await Axios.request<T>({
method: options.method,
url,
data: options.body,
params: options.query,
headers: {
'kbn-xsrf': 'kbn-client',
},
});

return response.data;
} catch (error) {
let retryErrorMsg: string | undefined;
if (isAxiosRequestError(error)) {
retryErrorMsg = `[${description}] request failed (attempt=${attempt})`;
} else if (isConcliftOnGetError(error)) {
retryErrorMsg = `Conflict on GET (path=${options.path}, attempt=${attempt})`;
}
let attempt = 0;
const maxAttempts = options.retries ?? DEFAULT_MAX_ATTEMPTS;

while (true) {
attempt += 1;

try {
const response = await Axios.request<T>({
method: options.method,
url,
data: options.body,
params: options.query,
headers: {
'kbn-xsrf': 'kbn-client',
},
});

return response.data;
} catch (error) {
const conflictOnGet = isConcliftOnGetError(error);
const requestedRetries = options.retries !== undefined;
const failedToGetResponse = isAxiosRequestError(error);

let errorMessage;
if (conflictOnGet) {
errorMessage = `Conflict on GET (path=${options.path}, attempt=${attempt}/${maxAttempts})`;
this.log.error(errorMessage);
} else if (requestedRetries || failedToGetResponse) {
errorMessage = `[${description}] request failed (attempt=${attempt}/${maxAttempts})`;
this.log.error(errorMessage);
} else {
throw error;
}

if (retryErrorMsg) {
if (attempt < maxAttempts) {
this.log.error(retryErrorMsg);
await delay(1000 * attempt);
return await this.request<T>({
...options,
attempt: attempt + 1,
});
continue;
}

throw new Error(retryErrorMsg + ' and ran out of retries');
throw new Error(`${errorMessage} -- and ran out of retries`);
}

throw error;
}
}
}
30 changes: 16 additions & 14 deletions packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class KbnClientUiSettings {

async get(setting: string) {
const all = await this.getAll();
const value = all.settings[setting] ? all.settings[setting].userValue : undefined;
const value = all[setting]?.userValue;

this.log.verbose('uiSettings.value: %j', value);
return value;
Expand Down Expand Up @@ -68,24 +68,24 @@ export class KbnClientUiSettings {
* with some defaults
*/
async replace(doc: UiSettingValues) {
const all = await this.getAll();
for (const [name, { isOverridden }] of Object.entries(all.settings)) {
if (!isOverridden) {
await this.unset(name);
this.log.debug('replacing kibana config doc: %j', doc);

const changes: Record<string, any> = {
...this.defaults,
...doc,
};

for (const [name, { isOverridden }] of Object.entries(await this.getAll())) {
if (!isOverridden && !changes.hasOwnProperty(name)) {
changes[name] = null;
}
}

this.log.debug('replacing kibana config doc: %j', doc);

await this.requester.request({
method: 'POST',
path: '/api/kibana/settings',
body: {
changes: {
...this.defaults,
...doc,
},
},
body: { changes },
retries: 5,
});
}

Expand All @@ -105,9 +105,11 @@ export class KbnClientUiSettings {
}

private async getAll() {
return await this.requester.request<UiSettingsApiResponse>({
const resp = await this.requester.request<UiSettingsApiResponse>({
path: '/api/kibana/settings',
method: 'GET',
});

return resp.settings;
}
}
1 change: 1 addition & 0 deletions packages/kbn-dev-utils/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"extends": "../../tsconfig.json",
"compilerOptions": {
"outDir": "target",
"target": "ES2019",
"declaration": true
},
"include": [
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-test/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,5 @@ export {
} from './mocha';

export { runFailedTestsReporterCli } from './failed_tests_reporter';

export { makeJunitReportPath } from './junit_report_path';
32 changes: 32 additions & 0 deletions packages/kbn-test/src/junit_report_path.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { resolve } from 'path';

const job = process.env.JOB ? `job-${process.env.JOB}-` : '';
const num = process.env.CI_WORKER_NUMBER ? `worker-${process.env.CI_WORKER_NUMBER}-` : '';

export function makeJunitReportPath(rootDirectory: string, reportName: string) {
return resolve(
rootDirectory,
'target/junit',
process.env.JOB || '.',
`TEST-${job}${num}${reportName}.xml`
);
}
13 changes: 2 additions & 11 deletions packages/kbn-test/src/mocha/__tests__/junit_report_generation.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { parseString } from 'xml2js';
import del from 'del';
import Mocha from 'mocha';
import expect from '@kbn/expect';
import { makeJunitReportPath } from '@kbn/test';

import { setupJUnitReportGeneration } from '../junit_report_generation';

Expand All @@ -50,17 +51,7 @@ describe('dev/mocha/junit report generation', () => {
mocha.addFile(resolve(PROJECT_DIR, 'test.js'));
await new Promise(resolve => mocha.run(resolve));
const report = await fcb(cb =>
parseString(
readFileSync(
resolve(
PROJECT_DIR,
'target/junit',
process.env.JOB || '.',
`TEST-${process.env.JOB ? process.env.JOB + '-' : ''}test.xml`
)
),
cb
)
parseString(readFileSync(makeJunitReportPath(PROJECT_DIR, 'test')), cb)
);

// test case results are wrapped in <testsuites></testsuites>
Expand Down
11 changes: 3 additions & 8 deletions packages/kbn-test/src/mocha/junit_report_generation.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
* under the License.
*/

import { resolve, dirname, relative } from 'path';
import { dirname, relative } from 'path';
import { writeFileSync, mkdirSync } from 'fs';
import { inspect } from 'util';

import xmlBuilder from 'xmlbuilder';
import { makeJunitReportPath } from '@kbn/test';

import { getSnapshotOfRunnableLogs } from './log_cache';
import { escapeCdata } from '../';
Expand Down Expand Up @@ -137,13 +138,7 @@ export function setupJUnitReportGeneration(runner, options = {}) {
}
});

const reportPath = resolve(
rootDirectory,
'target/junit',
process.env.JOB || '.',
`TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml`
);

const reportPath = makeJunitReportPath(rootDirectory, reportName);
const reportXML = builder.end();
mkdirSync(dirname(reportPath), { recursive: true });
writeFileSync(reportPath, reportXML, 'utf8');
Expand Down
3 changes: 2 additions & 1 deletion src/dev/jest/integration_tests/junit_reporter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ import { readFileSync } from 'fs';
import del from 'del';
import execa from 'execa';
import xml2js from 'xml2js';
import { makeJunitReportPath } from '@kbn/test';

const MINUTE = 1000 * 60;
const ROOT_DIR = resolve(__dirname, '../../../../');
const FIXTURE_DIR = resolve(__dirname, '__fixtures__');
const TARGET_DIR = resolve(FIXTURE_DIR, 'target');
const XML_PATH = resolve(TARGET_DIR, 'junit', process.env.JOB || '.', `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}Jest Tests.xml`);
const XML_PATH = makeJunitReportPath(FIXTURE_DIR, 'Jest Tests');

afterAll(async () => {
await del(TARGET_DIR);
Expand Down
10 changes: 2 additions & 8 deletions src/dev/jest/junit_reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { writeFileSync, mkdirSync } from 'fs';

import xmlBuilder from 'xmlbuilder';

import { escapeCdata } from '@kbn/test';
import { escapeCdata, makeJunitReportPath } from '@kbn/test';

const ROOT_DIR = dirname(require.resolve('../../../package.json'));

Expand Down Expand Up @@ -102,13 +102,7 @@ export default class JestJUnitReporter {
});
});

const reportPath = resolve(
rootDirectory,
'target/junit',
process.env.JOB || '.',
`TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml`
);

const reportPath = makeJunitReportPath(rootDirectory, reportName);
const reportXML = root.end();
mkdirSync(dirname(reportPath), { recursive: true });
writeFileSync(reportPath, reportXML, 'utf8');
Expand Down
5 changes: 3 additions & 2 deletions tasks/config/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* under the License.
*/

import { resolve, dirname } from 'path';
import { dirname } from 'path';
import { times } from 'lodash';
import { makeJunitReportPath } from '@kbn/test';

const TOTAL_CI_SHARDS = 4;
const ROOT = dirname(require.resolve('../../package.json'));
Expand Down Expand Up @@ -79,7 +80,7 @@ module.exports = function (grunt) {
reporters: pickReporters(),

junitReporter: {
outputFile: resolve(ROOT, 'target/junit', process.env.JOB || '.', `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}karma.xml`),
outputFile: makeJunitReportPath(ROOT, 'karma'),
useBrowserName: false,
nameFormatter: (_, result) => [...result.suite, result.description].join(' '),
classNameFormatter: (_, result) => {
Expand Down

0 comments on commit ab1fe3f

Please sign in to comment.