Skip to content

Commit cc23912

Browse files
author
Spencer
authored
[7.x] [kbnClient] Retry uiSettings.replace() calls up to 5 tim… (#52779)
* [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 # Conflicts: # src/dev/jest/junit_reporter.js
1 parent 65a6e8b commit cc23912

10 files changed

Lines changed: 99 additions & 77 deletions

File tree

‎packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts‎

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ export interface ReqOptions {
6262
query?: Record<string, any>;
6363
method: 'GET' | 'POST' | 'PUT' | 'DELETE';
6464
body?: any;
65-
attempt?: number;
66-
maxAttempts?: number;
65+
retries?: number;
6766
}
6867

6968
const delay = (ms: number) =>
@@ -87,44 +86,47 @@ export class KbnClientRequester {
8786
async request<T>(options: ReqOptions): Promise<T> {
8887
const url = Url.resolve(this.pickUrl(), options.path);
8988
const description = options.description || `${options.method} ${url}`;
90-
const attempt = options.attempt === undefined ? 1 : options.attempt;
91-
const maxAttempts =
92-
options.maxAttempts === undefined ? DEFAULT_MAX_ATTEMPTS : options.maxAttempts;
93-
94-
try {
95-
const response = await Axios.request<T>({
96-
method: options.method,
97-
url,
98-
data: options.body,
99-
params: options.query,
100-
headers: {
101-
'kbn-xsrf': 'kbn-client',
102-
},
103-
});
104-
105-
return response.data;
106-
} catch (error) {
107-
let retryErrorMsg: string | undefined;
108-
if (isAxiosRequestError(error)) {
109-
retryErrorMsg = `[${description}] request failed (attempt=${attempt})`;
110-
} else if (isConcliftOnGetError(error)) {
111-
retryErrorMsg = `Conflict on GET (path=${options.path}, attempt=${attempt})`;
112-
}
89+
let attempt = 0;
90+
const maxAttempts = options.retries ?? DEFAULT_MAX_ATTEMPTS;
91+
92+
while (true) {
93+
attempt += 1;
94+
95+
try {
96+
const response = await Axios.request<T>({
97+
method: options.method,
98+
url,
99+
data: options.body,
100+
params: options.query,
101+
headers: {
102+
'kbn-xsrf': 'kbn-client',
103+
},
104+
});
105+
106+
return response.data;
107+
} catch (error) {
108+
const conflictOnGet = isConcliftOnGetError(error);
109+
const requestedRetries = options.retries !== undefined;
110+
const failedToGetResponse = isAxiosRequestError(error);
111+
112+
let errorMessage;
113+
if (conflictOnGet) {
114+
errorMessage = `Conflict on GET (path=${options.path}, attempt=${attempt}/${maxAttempts})`;
115+
this.log.error(errorMessage);
116+
} else if (requestedRetries || failedToGetResponse) {
117+
errorMessage = `[${description}] request failed (attempt=${attempt}/${maxAttempts})`;
118+
this.log.error(errorMessage);
119+
} else {
120+
throw error;
121+
}
113122

114-
if (retryErrorMsg) {
115123
if (attempt < maxAttempts) {
116-
this.log.error(retryErrorMsg);
117124
await delay(1000 * attempt);
118-
return await this.request<T>({
119-
...options,
120-
attempt: attempt + 1,
121-
});
125+
continue;
122126
}
123127

124-
throw new Error(retryErrorMsg + ' and ran out of retries');
128+
throw new Error(`${errorMessage} -- and ran out of retries`);
125129
}
126-
127-
throw error;
128130
}
129131
}
130132
}

‎packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export class KbnClientUiSettings {
4040

4141
async get(setting: string) {
4242
const all = await this.getAll();
43-
const value = all.settings[setting] ? all.settings[setting].userValue : undefined;
43+
const value = all[setting]?.userValue;
4444

4545
this.log.verbose('uiSettings.value: %j', value);
4646
return value;
@@ -68,24 +68,24 @@ export class KbnClientUiSettings {
6868
* with some defaults
6969
*/
7070
async replace(doc: UiSettingValues) {
71-
const all = await this.getAll();
72-
for (const [name, { isOverridden }] of Object.entries(all.settings)) {
73-
if (!isOverridden) {
74-
await this.unset(name);
71+
this.log.debug('replacing kibana config doc: %j', doc);
72+
73+
const changes: Record<string, any> = {
74+
...this.defaults,
75+
...doc,
76+
};
77+
78+
for (const [name, { isOverridden }] of Object.entries(await this.getAll())) {
79+
if (!isOverridden && !changes.hasOwnProperty(name)) {
80+
changes[name] = null;
7581
}
7682
}
7783

78-
this.log.debug('replacing kibana config doc: %j', doc);
79-
8084
await this.requester.request({
8185
method: 'POST',
8286
path: '/api/kibana/settings',
83-
body: {
84-
changes: {
85-
...this.defaults,
86-
...doc,
87-
},
88-
},
87+
body: { changes },
88+
retries: 5,
8989
});
9090
}
9191

@@ -105,9 +105,11 @@ export class KbnClientUiSettings {
105105
}
106106

107107
private async getAll() {
108-
return await this.requester.request<UiSettingsApiResponse>({
108+
const resp = await this.requester.request<UiSettingsApiResponse>({
109109
path: '/api/kibana/settings',
110110
method: 'GET',
111111
});
112+
113+
return resp.settings;
112114
}
113115
}

‎packages/kbn-dev-utils/tsconfig.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"extends": "../../tsconfig.json",
33
"compilerOptions": {
44
"outDir": "target",
5+
"target": "ES2019",
56
"declaration": true
67
},
78
"include": [

‎packages/kbn-test/src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,5 @@ export { readConfigFile } from './functional_test_runner/lib/config/read_config_
4242
export { runFtrCli } from './functional_test_runner/cli';
4343

4444
export { runFailedTestsReporterCli } from './failed_tests_reporter';
45+
46+
export { makeJunitReportPath } from './junit_report_path';

‎packages/kbn-test/src/junit_report_path.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import { resolve } from 'path';
21+
22+
const job = process.env.JOB ? `job-${process.env.JOB}-` : '';
23+
const num = process.env.CI_WORKER_NUMBER ? `worker-${process.env.CI_WORKER_NUMBER}-` : '';
24+
25+
export function makeJunitReportPath(rootDirectory: string, reportName: string) {
26+
return resolve(
27+
rootDirectory,
28+
'target/junit',
29+
process.env.JOB || '.',
30+
`TEST-${job}${num}${reportName}.xml`
31+
);
32+
}

‎src/dev/jest/integration_tests/junit_reporter.test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,13 @@ import { readFileSync } from 'fs';
2424
import del from 'del';
2525
import execa from 'execa';
2626
import xml2js from 'xml2js';
27+
import { makeJunitReportPath } from '@kbn/test';
2728

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

3435
afterAll(async () => {
3536
await del(TARGET_DIR);

‎src/dev/jest/junit_reporter.js‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { writeFileSync, mkdirSync } from 'fs';
2323
import xmlBuilder from 'xmlbuilder';
2424

2525
import { escapeCdata } from '../xml';
26+
import { makeJunitReportPath } from '@kbn/test';
2627

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

@@ -102,13 +103,7 @@ export default class JestJUnitReporter {
102103
});
103104
});
104105

105-
const reportPath = resolve(
106-
rootDirectory,
107-
'target/junit',
108-
process.env.JOB || '.',
109-
`TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml`
110-
);
111-
106+
const reportPath = makeJunitReportPath(rootDirectory, reportName);
112107
const reportXML = root.end();
113108
mkdirSync(dirname(reportPath), { recursive: true });
114109
writeFileSync(reportPath, reportXML, 'utf8');

‎src/dev/mocha/__tests__/junit_report_generation.js‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { parseString } from 'xml2js';
2525
import del from 'del';
2626
import Mocha from 'mocha';
2727
import expect from '@kbn/expect';
28+
import { makeJunitReportPath } from '@kbn/test';
2829

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

@@ -50,17 +51,7 @@ describe('dev/mocha/junit report generation', () => {
5051
mocha.addFile(resolve(PROJECT_DIR, 'test.js'));
5152
await new Promise(resolve => mocha.run(resolve));
5253
const report = await fcb(cb =>
53-
parseString(
54-
readFileSync(
55-
resolve(
56-
PROJECT_DIR,
57-
'target/junit',
58-
process.env.JOB || '.',
59-
`TEST-${process.env.JOB ? process.env.JOB + '-' : ''}test.xml`
60-
)
61-
),
62-
cb
63-
)
54+
parseString(readFileSync(makeJunitReportPath(PROJECT_DIR, 'test')), cb)
6455
);
6556

6657
// test case results are wrapped in <testsuites></testsuites>

‎src/dev/mocha/junit_report_generation.js‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@
1717
* under the License.
1818
*/
1919

20-
import { resolve, dirname, relative } from 'path';
20+
import { dirname, relative } from 'path';
2121
import { writeFileSync, mkdirSync } from 'fs';
2222
import { inspect } from 'util';
2323

2424
import xmlBuilder from 'xmlbuilder';
25+
import { makeJunitReportPath } from '@kbn/test';
2526

2627
import { getSnapshotOfRunnableLogs } from './log_cache';
2728
import { escapeCdata } from '../xml';
@@ -137,13 +138,7 @@ export function setupJUnitReportGeneration(runner, options = {}) {
137138
}
138139
});
139140

140-
const reportPath = resolve(
141-
rootDirectory,
142-
'target/junit',
143-
process.env.JOB || '.',
144-
`TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml`
145-
);
146-
141+
const reportPath = makeJunitReportPath(rootDirectory, reportName);
147142
const reportXML = builder.end();
148143
mkdirSync(dirname(reportPath), { recursive: true });
149144
writeFileSync(reportPath, reportXML, 'utf8');

‎tasks/config/karma.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@
1717
* under the License.
1818
*/
1919

20-
import { resolve, dirname } from 'path';
20+
import { dirname } from 'path';
2121
import { times } from 'lodash';
22+
import { makeJunitReportPath } from '@kbn/test';
2223

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

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

0 commit comments

Comments
 (0)