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

[MDS] Use DataSourceError to replace Error in getDataSourceById call #6903

Merged
merged 9 commits into from
Jun 6, 2024
Merged

[MDS] Use DataSourceError to replace Error in getDataSourceById call #6903

merged 9 commits into from
Jun 6, 2024

Conversation

yujin-emma
Copy link
Contributor

Description

Use DataSourceError to replace Error in getDataSourceById call

Issues Resolved

#6738

Screenshot

 Get data source by ID
      ✓ Success: getting data source by ID with credential (1 ms)
      ✓ Success: getting data source by ID without credential
      ✓ Success but no data: getting data source by ID without credential (1 ms)
      ✓ failure: getting data source by ID
      ✓ failure: gets error when response contains not found error
      ✓ failure: gets error when response contains other error

Testing the changes

Changelog

  • fix: Use DataSourceError to replace Error in getDataSourceById call

Check List

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

opensearch-changeset-bot bot added a commit to yujin-emma/OpenSearch-Dashboards that referenced this pull request Jun 4, 2024
@opensearch-changeset-bot
Changeset file for PR opensearch-project#6903 created/updated
92fbcfc
@codecov Codecov
Copy link

codecov bot commented Jun 4, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.

Project coverage is 67.41%. Comparing base (9674147) to head (b34080f).

Files Patch % Lines
.../data_source_management/public/components/utils.ts 0.00% 4 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6903      +/-   ##
==========================================
- Coverage   67.45%   67.41%   -0.04%     
==========================================
  Files        3443     3444       +1     
  Lines       67815    67819       +4     
  Branches    11033    11034       +1     
==========================================
- Hits        45742    45723      -19     
- Misses      19406    19430      +24     
+ Partials     2667     2666       -1
Flag Coverage Δ
Linux_1 33.09% <ø> (ø)
Linux_2 55.06% <ø> (ø)
Linux_3 45.20% <80.95%> (+<0.01%) ⬆️
Linux_4 34.87% <5.26%> (-0.02%) ⬇️
Windows_1 ?
Windows_2 55.01% <ø> (ø)
Windows_3 45.21% <80.95%> (-0.01%) ⬇️
Windows_4 34.87% <5.26%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yujin-emma yujin-emma marked this pull request as ready for review June 4, 2024 16:42
@bandinib-amzn
Copy link
Member

Hi @yujin-emma Thanks for the change. Can you please add more UT to pass codecov check?

bandinib-amzn
bandinib-amzn reviewed Jun 5, 2024
View reviewed changes
src/plugins/data_source/common/data_sources/error.ts Show resolved Hide resolved
src/plugins/data_source_management/public/components/utils.test.ts Outdated
);
await getDataSourceById('alpha-test', savedObjects.client);
} catch (e) {
// expect(e).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove comment

Copy link
Contributor Author

@yujin-emma yujin-emma Jun 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

updated

@yujin-emma
Copy link
Contributor Author

Found the test coverage failed for the move, the test already exist in the old folder, and the test there all pass

@BionIT
Copy link
Collaborator

BionIT commented Jun 6, 2024

@yujin-emma could you fix the test failures?

yujin-emma and others added 2 commits June 6, 2024 07:40
@yujin-emma
Merge branch 'main' into getds-res-404
d5a59ab
@yujin-emma
Fix failed integration test
5937da4 
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
@yujin-emma
Copy link
Contributor Author

@yujin-emma could you fix the test failures?

Fixed

@ZilongX
Copy link
Collaborator

ZilongX commented Jun 6, 2024

Picked up latest main and re-running checks

@zhongnansu zhongnansu merged commit fbb8973 into opensearch-project:main Jun 6, 2024
67 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-6903-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fbb89736e390e825f669ac49def4a529695c1cf8
# Push it to GitHub
git push --set-upstream origin backport/backport-6903-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-6903-to-2.x.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-6903-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fbb89736e390e825f669ac49def4a529695c1cf8
# Push it to GitHub
git push --set-upstream origin backport/backport-6903-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-6903-to-2.x.

yujin-emma added a commit to yujin-emma/OpenSearch-Dashboards that referenced this pull request Jun 6, 2024
@yujin-emma
Manual backport opensearch-project#6903
b10f8bb 
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
@yujin-emma yujin-emma mentioned this pull request Jun 6, 2024
Merged
7 tasks
xinruiba pushed a commit that referenced this pull request Jun 6, 2024
@yujin-emma
Manual backport #6903 (#6952)
c0d0b7e 
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
@BionIT
Copy link
Collaborator

BionIT commented Jun 6, 2024

@yujin-emma is it safe to remove the failed backport label?

@yujin-emma
Copy link
Contributor Author

@yujin-emma is it safe to remove the failed backport label?

I removed after the manual backport PR merged

@yujin-emma yujin-emma deleted the getds-res-404 branch June 6, 2024 20:16
@BionIT BionIT mentioned this pull request Jun 7, 2024
Closed
45 tasks
@zhongnansu zhongnansu added the multiple datasource multiple datasource project label Jun 10, 2024
@zhongnansu zhongnansu mentioned this pull request Jun 10, 2024
Merged
7 tasks
@zhyuanqi zhyuanqi added the enhancement New feature or request label Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xinruiba xinruiba xinruiba approved these changes

@bandinib-amzn bandinib-amzn bandinib-amzn approved these changes

@ananzh ananzh Awaiting requested review from ananzh ananzh is a code owner

@kavilla kavilla Awaiting requested review from kavilla kavilla is a code owner

@AMoo-Miki AMoo-Miki Awaiting requested review from AMoo-Miki AMoo-Miki is a code owner

@ashwin-pc ashwin-pc Awaiting requested review from ashwin-pc ashwin-pc is a code owner

@joshuarrrr joshuarrrr Awaiting requested review from joshuarrrr joshuarrrr is a code owner

@abbyhu2000 abbyhu2000 Awaiting requested review from abbyhu2000 abbyhu2000 is a code owner

@zengyan-amazon zengyan-amazon Awaiting requested review from zengyan-amazon zengyan-amazon is a code owner

@zhongnansu zhongnansu Awaiting requested review from zhongnansu zhongnansu is a code owner

@manasvinibs manasvinibs Awaiting requested review from manasvinibs manasvinibs is a code owner

@ZilongX ZilongX Awaiting requested review from ZilongX ZilongX is a code owner

@Flyingliuhub Flyingliuhub Awaiting requested review from Flyingliuhub Flyingliuhub is a code owner

@curq curq Awaiting requested review from curq curq is a code owner

@SuZhou-Joe SuZhou-Joe Awaiting requested review from SuZhou-Joe SuZhou-Joe is a code owner

@ruanyl ruanyl Awaiting requested review from ruanyl ruanyl is a code owner

@BionIT BionIT Awaiting requested review from BionIT BionIT is a code owner

@zhyuanqi zhyuanqi Awaiting requested review from zhyuanqi zhyuanqi is a code owner

@mengweieric mengweieric Awaiting requested review from mengweieric mengweieric is a code owner

Assignees

@bandinib-amzn bandinib-amzn

@xinruiba xinruiba

Labels
backport 2.x enhancement New feature or request multiple datasource multiple datasource project seasoned-contributor v2.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@yujin-emma @bandinib-amzn @BionIT @ZilongX @xinruiba @zhongnansu @zhyuanqi