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

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

Merged
merged 2 commits into from
Oct 6, 2022

Conversation

zhongnansu
Copy link
Member

@zhongnansu zhongnansu commented Oct 5, 2022

Signed-off-by: Su szhongna@amazon.com

Description

Display error toast for create index pattern with data source.
Make use of the existing toast component instead of swallowing error, we generate the pop-up for data source use case in index pattern
image

Issues Resolved

#2285

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Su <szhongna@amazon.com>
@zhongnansu zhongnansu requested a review from a team as a code owner October 5, 2022 00:53
@zhongnansu zhongnansu added the multiple datasource multiple datasource project label Oct 5, 2022
@zhongnansu zhongnansu self-assigned this Oct 5, 2022
Signed-off-by: Su <szhongna@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2022

Codecov Report

Merging #2506 (558f0f9) into main (b75e07d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2506   +/-   ##
=======================================
  Coverage   66.78%   66.78%           
=======================================
  Files        3201     3201           
  Lines       60913    60915    +2     
  Branches     9254     9255    +1     
=======================================
+ Hits        40681    40683    +2     
  Misses      18019    18019           
  Partials     2213     2213           
Impacted Files Coverage Δ
...dex_pattern_wizard/create_index_pattern_wizard.tsx 67.08% <ø> (ø)
...nts/create_index_pattern_wizard/lib/get_indices.ts 79.54% <100.00%> (+0.47%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@joshuarrrr joshuarrrr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@noCharger noCharger left a comment

Choose a reason for hiding this comment

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

Approved with comments: Do we need UX input for the error message?

@@ -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.

@kristenTian kristenTian merged commit 0e742b9 into opensearch-project:main Oct 6, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 6, 2022
…2506)

* add error toast for create index pattern with data source

Signed-off-by: Su <szhongna@amazon.com>

* update change log

Signed-off-by: Su <szhongna@amazon.com>

Signed-off-by: Su <szhongna@amazon.com>
(cherry picked from commit 0e742b9)
kavilla pushed a commit that referenced this pull request Oct 11, 2022
…2506) (#2526)

* add error toast for create index pattern with data source
* update change log

Signed-off-by: Su <szhongna@amazon.com>
(cherry picked from commit 0e742b9)
@ananzh ananzh added the enhancement New feature or request label Nov 5, 2022
@AMoo-Miki AMoo-Miki added the v2.4.0 'Issues and PRs related to version v2.4.0' label Nov 5, 2022
pjfitzgibbons pushed a commit to pjfitzgibbons/OpenSearch-Dashboards that referenced this pull request Dec 1, 2022
…pensearch-project#2506) (opensearch-project#2526)

* add error toast for create index pattern with data source
* update change log

Signed-off-by: Su <szhongna@amazon.com>
(cherry picked from commit 0e742b9)
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this pull request Dec 16, 2022
…pensearch-project#2506)

* add error toast for create index pattern with data source

Signed-off-by: Su <szhongna@amazon.com>

* update change log

Signed-off-by: Su <szhongna@amazon.com>

Signed-off-by: Su <szhongna@amazon.com>
Signed-off-by: Sergey V. Osipov <sipopo@yandex.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request multiple datasource multiple datasource project v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants