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.

Sign up for GitHub

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

Jump to bottom

[MD] Display error toast for create index pattern with data source #2506

Merged
merged 2 commits into from
Oct 6, 2022
Merged

[MD] Display error toast for create index pattern with data source #2506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7abac0
add error toast for create index pattern with data source
zhongnansu Oct 5, 2022
558f0f9
update change log
zhongnansu Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

* [MD] Support legacy client for data source ([#2204](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2204))
* [Plugin Helpers] Facilitate version changes ([#2398](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2398))
* [MD] Display error toast for create index pattern with data source ([#2506](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2506))

### 🐛 Bug Fixes
* [Vis Builder] Fixes auto bounds for timeseries bar chart visualization ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401))
Expand Down
1 change: 1 addition & 0 deletions ..._management/public/components/create_index_pattern_wizard/create_index_pattern_wizard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ export class CreateIndexPatternWizard extends Component<
id: errorMsg.props.id,
color: 'warning',
iconType: 'alert',
text: errors.body.message,
},
]),
}));
Expand Down
18 changes: 16 additions & 2 deletions ..._pattern_management/public/components/create_index_pattern_wizard/lib/get_indices.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe('getIndices', () => {
expect(result[1].name).toBe('foo');
});

it('should make two calls in cross cluser case', async () => {
it('should make two calls in cross cluster case', async () => {
http.get.mockResolvedValue(successfulResolveResponse);
const result = await getIndices({
http,
Expand Down Expand Up @@ -201,9 +201,23 @@ describe('getIndices', () => {
getIndexTags,
pattern: 'opensearch-dashboards',
searchClient,
dataSourceId,
});
expect(result.length).toBe(0);
});

it('should throw error with data source use case', async () => {
http.get.mockImplementationOnce(() => {
throw new Error('Test error');
});
expect(
getIndices({
http,
getIndexTags,
pattern: 'opensearch-dashboards',
searchClient,
dataSourceId,
})
).rejects.toThrowError();
});
});
});
9 changes: 8 additions & 1 deletion ...index_pattern_management/public/components/create_index_pattern_wizard/lib/get_indices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,14 @@ export async function getIndices({
pattern,
showAllIndices,
dataSourceId,
}).catch(() => []);
}).catch((error) => {
// swallow the errors to be backwards compatible with non-data-source use case
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is also line 103 could use the same :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 103 maps to getIndicesViaSearch which is specifically for CCS use case as a step 2 request. But for CCS it also requires step 1 querying against local cluster. Even we added the same logic to step 2, the code won't be able to reach there, because the error will be thrown at step 1 already. Let's leave as it is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point was the call through search client in getIndicesViaSearch itself could also have error. And it's a different route from the resolve api. But it's not blocking this PR though.

if (dataSourceId) {
throw error;
} else {
return [];
}
});
requests.push(promiseResolve);

if (isCCS) {
Expand Down