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

[workspace]add 2 step loading in 'Associate data sources' modal #8999

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

[workspace]add 2 step loading in 'Associate data sources' modal #8999

wants to merge 7 commits into from

Conversation

Qxisylolo
Copy link
Contributor

@Qxisylolo Qxisylolo commented Dec 3, 2024 •
edited
Loading

Description

This pr addresses the issue where a single data source has numerous connections, causing it to take more than 20 seconds to list the available data sources.
When a data source has numerous dqc connections, it will display the data source first with a badge 'loading' which will be replaced by '+n related' after dqc connections are completely loaded

Screenshot

2024-12-11.17.46.05.mov

Testing the changes

Changelog

  • feat: Add two-steps loading for associating data sources

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

@Qxisylolo
add 2 step loading
1ed0922 
Signed-off-by: Qxisylolo <[email protected]>
@Qxisylolo Qxisylolo changed the title add 2 step loading [workspace]add 2 step loading in 'Associate data sources' modal Dec 3, 2024
@codecov Codecov
Copy link

codecov bot commented Dec 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.

Project coverage is 60.87%. Comparing base (473c0ae) to head (9b4259d).
Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
src/plugins/workspace/public/utils.ts 16.66% 5 Missing ⚠️
...urce_association/association_data_source_modal.tsx 96.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8999      +/-   ##
==========================================
- Coverage   60.88%   60.87%   -0.02%     
==========================================
  Files        3802     3808       +6     
  Lines       91072    91233     +161     
  Branches    14375    14413      +38     
==========================================
+ Hits        55447    55534      +87     
- Misses      32083    32158      +75     
+ Partials     3542     3541       -1
Flag Coverage Δ
Linux_1 29.03% <83.78%> (+0.01%) ⬆️
Linux_2 56.38% <ø> (ø)
Linux_3 37.93% <ø> (+0.03%) ⬆️
Linux_4 29.01% <ø> (+0.01%) ⬆️
Windows_1 29.05% <83.78%> (+0.01%) ⬆️
Windows_2 56.34% <ø> (ø)
Windows_3 37.94% <ø> (+0.03%) ⬆️
Windows_4 29.01% <ø> (+0.01%) ⬆️

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.

@Qxisylolo
ifx ut
df5de09 
Signed-off-by: Qxisylolo <[email protected]>
@Qxisylolo Qxisylolo marked this pull request as ready for review December 4, 2024 02:19
@Qxisylolo Qxisylolo requested a review from huyaboo as a code owner December 4, 2024 02:19
@Qxisylolo
resolve comments
feaba8b 
Signed-off-by: Qxisylolo <[email protected]>
wanglam
wanglam reviewed Dec 5, 2024
View reviewed changes
...lugins/workspace/public/components/data_source_association/association_data_source_modal.tsx Outdated
return { openSearchConnections, dataConnections, dataSourcesList };
})
.then(({ openSearchConnections, dataConnections, dataSourcesList }) => {
fetchDirectQueryConnections(dataSourcesList, http, notifications).then(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I'm thinking of do we need to have a loading status for direct query connections for each data source. Since load direct query connections may take long time, but if we separate to each data source, it would be more faster.

Copy link
Contributor Author

@Qxisylolo Qxisylolo Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the advice, updated and new screenshot is also updated

@Qxisylolo
delete functions
ee0299f 
Signed-off-by: Qxisylolo <[email protected]>
@Qxisylolo
separately fetch dqc
c506daa 
Signed-off-by: Qxisylolo <[email protected]>
@Qxisylolo Qxisylolo force-pushed the feat/delay_in-_loading_data_sources branch from 83c1820 to a3c139c Compare December 11, 2024 10:26
@Qxisylolo
small mistakes
9b4259d 
Signed-off-by: Qxisylolo <[email protected]>
@Qxisylolo Qxisylolo force-pushed the feat/delay_in-_loading_data_sources branch from a3c139c to 9b4259d Compare December 11, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raintygao raintygao raintygao left review comments

@wanglam wanglam wanglam left review comments

@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

@bandinib-amzn bandinib-amzn Awaiting requested review from bandinib-amzn bandinib-amzn 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

@xinruiba xinruiba Awaiting requested review from xinruiba xinruiba 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

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

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

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

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

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

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
seasoned-contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Qxisylolo @wanglam @raintygao