Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
rshen91 committed Nov 16, 2023
1 parent 6013b2d commit 9950af0
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 56 deletions.
42 changes: 5 additions & 37 deletions packages/kbn-generate-csv/src/generate_csv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ describe('CsvGenerator', () => {
maxSizeBytes: 180000,
useByteOrderMarkEncoding: false,
scroll: { size: 500, duration: '30s' },
maxConcurrentShardRequests: 5,
enablePanelActionDownload: true,
maxConcurrentShardRequests: 5,
};

searchSourceMock.getField = jest.fn((key: string) => {
Expand Down Expand Up @@ -245,8 +245,8 @@ describe('CsvGenerator', () => {
maxSizeBytes: TEST_MAX_SIZE,
useByteOrderMarkEncoding: false,
scroll: { size: 500, duration: '30s' },
maxConcurrentShardRequests: 5,
enablePanelActionDownload: true,
maxConcurrentShardRequests: 5,
};

mockDataClient.search = jest.fn().mockImplementation(() =>
Expand Down Expand Up @@ -347,7 +347,7 @@ describe('CsvGenerator', () => {

expect(mockDataClient.search).toHaveBeenCalledTimes(10);
expect(mockDataClient.search).toBeCalledWith(
{ params: { body: {}, ignore_throttled: undefined } },
{ params: { body: {}, ignore_throttled: undefined }, maxConcurrentShardRequests: 5 },
{ strategy: 'es', transport: { maxRetries: 0, requestTimeout: '30s' } }
);

Expand Down Expand Up @@ -760,8 +760,8 @@ describe('CsvGenerator', () => {
maxSizeBytes: 180000,
useByteOrderMarkEncoding: false,
scroll: { size: 500, duration: '30s' },
maxConcurrentShardRequests: 5,
enablePanelActionDownload: true,
maxConcurrentShardRequests: 5,
};
mockDataClient.search = jest.fn().mockImplementation(() =>
Rx.of({
Expand Down Expand Up @@ -847,6 +847,7 @@ describe('CsvGenerator', () => {

expect(mockDataClient.search).toBeCalledWith(
{
maxConcurrentShardRequests: 5,
params: {
body: {},
},
Expand Down Expand Up @@ -1054,37 +1055,4 @@ describe('CsvGenerator', () => {
`);
});
});

it('handles max concurrent shards settings set to a number', async () => {
const mockShardsConfig = {
...mockConfig,
maxConcurrentShardRequests: 6,
};
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockShardsConfig,
{
es: mockEsClient,
data: mockDataClient,
uiSettings: uiSettingsClient,
},
{
searchSourceStart: mockSearchSourceService,
fieldFormatsRegistry: mockFieldFormatsRegistry,
},
new CancellationToken(),
mockLogger,
stream
);
await generateCsv.generateData();
await expect(mockEsClient.asCurrentUser.openPointInTime).toHaveBeenCalledWith(
{
ignore_unavailable: true,
ignore_throttled: undefined,
index: 'logstash-*',
keep_alive: '30s',
},
{ maxConcurrentShardRequests: 6, maxRetries: 0, requestTimeout: '30s' }
);
});
});
23 changes: 10 additions & 13 deletions packages/kbn-generate-csv/src/generate_csv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ export class CsvGenerator {
private stream: Writable
) {}

private async openPointInTime(
indexPatternTitle: string,
settings: CsvExportSettings,
maxConcurrentShardRequests: number
) {
const { duration } = settings.scroll;
private async openPointInTime(indexPatternTitle: string, settings: CsvExportSettings) {
const {
includeFrozen,
maxConcurrentShardRequests,
scroll: { duration },
} = settings;
let pitId: string | undefined;
this.logger.debug(`Requesting PIT for: [${indexPatternTitle}]...`);
try {
Expand All @@ -77,7 +77,7 @@ export class CsvGenerator {
keep_alive: duration,
ignore_unavailable: true,
// @ts-expect-error ignore_throttled is not in the type definition, but it is accepted by es
ignore_throttled: settings.includeFrozen ? false : undefined, // "true" will cause deprecation warnings logged in ES
ignore_throttled: includeFrozen ? false : undefined, // "true" will cause deprecation warnings logged in ES
},
{
requestTimeout: duration,
Expand All @@ -104,7 +104,7 @@ export class CsvGenerator {
settings: CsvExportSettings,
searchAfter?: estypes.SortResults
) {
const { scroll: scrollSettings } = settings;
const { scroll: scrollSettings, maxConcurrentShardRequests } = settings;
searchSource.setField('size', scrollSettings.size);

if (searchAfter) {
Expand All @@ -126,6 +126,7 @@ export class CsvGenerator {
params: {
body: searchBody,
},
maxConcurrentShardRequests,
};

let results: estypes.SearchResponse<unknown> | undefined;
Expand Down Expand Up @@ -335,11 +336,7 @@ export class CsvGenerator {
let totalRelation = 'eq';
let searchAfter: estypes.SortResults | undefined;

let pitId = await this.openPointInTime(
indexPatternTitle,
settings,
this.config.maxConcurrentShardRequests
);
let pitId = await this.openPointInTime(indexPatternTitle, settings);

// apply timezone from the job to all date field formatters
try {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-generate-csv/src/get_export_settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('getExportSettings', () => {
"escapeFormulaValues": false,
"escapeValue": [Function],
"includeFrozen": false,
"maxConcurrentShardSize": 5,
"maxConcurrentShardRequests": 5,
"maxSizeBytes": 180000,
"scroll": Object {
"duration": "30s",
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-generate-csv/src/get_export_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export interface CsvExportSettings {
escapeFormulaValues: boolean;
escapeValue: (value: string) => string;
includeFrozen: boolean;
maxConcurrentShardSize: number;
maxConcurrentShardRequests: number;
}

export const getExportSettings = async (
Expand Down Expand Up @@ -83,6 +83,6 @@ export const getExportSettings = async (
checkForFormulas: config.checkForFormulas,
escapeFormulaValues,
escapeValue,
maxConcurrentShardSize: config.maxConcurrentShardRequests,
maxConcurrentShardRequests: config.maxConcurrentShardRequests,
};
};
1 change: 0 additions & 1 deletion packages/kbn-generate-csv/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,4 @@ export interface JobParamsCSV {
browserTimezone?: string;
searchSource: SerializedSearchSourceFields;
columns?: string[];
maxConcurrentShardRequests: number;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React, { useMemo, useCallback, useState } from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
import moment from 'moment';
import { encode } from '@kbn/rison';
import { i18n } from '@kbn/i18n';
Expand Down Expand Up @@ -301,7 +302,10 @@ export const LogEntryExampleMessageHeaders: React.FunctionComponent<{
data-test-subj="logColumnHeader messageLogColumnHeader"
key={columnConfiguration.messageColumn.id}
>
Message
<FormattedMessage
id="xpack.infra.logEntryExampleMessageHeaders.logColumnHeader.messageLabel"
defaultMessage="Message"
/>
</LogColumnHeader>
);
} else if (isFieldLogColumnConfiguration(columnConfiguration)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ export const CustomMetricForm = withTheme(
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText color="subdued">
<span>of</span>
<span>
{i18n.translate('xpack.infra.customMetricForm.span.ofLabel', {
defaultMessage: 'of',
})}
</span>
</EuiText>
</EuiFlexItem>
<EuiFlexItem>
Expand Down

0 comments on commit 9950af0

Please sign in to comment.