-
Notifications
You must be signed in to change notification settings - Fork 94
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
fix(list): Do not fetch directory URIs in stat
#3093
Conversation
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #3093 +/- ##
==========================================
+ Coverage 92.73% 92.77% +0.03%
==========================================
Files 113 113
Lines 11566 11608 +42
Branches 2553 2572 +19
==========================================
+ Hits 10726 10769 +43
+ Misses 838 837 -1
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Trae Yelovich <[email protected]>
stat
d36a62e
to
bf910c5
Compare
Signed-off-by: Trae Yelovich <[email protected]>
bf910c5
to
66d9fd3
Compare
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
📅 Suggested merge-by date: 9/20/2024 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes make sense to me! 🙏🏽
Thanks for addressing the issue so quickly! 🥳
LGTM! 😋
Signed-off-by: Trae Yelovich <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 😋
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works great and solves the hang mentioned on call last week retrieving data set members. Thanks @traeok for the fix!
Proposed changes
There was an oversight when implementing the remote lookup functionality for Data Sets and USS. The
stat
implementations should never make an API call unless we are explicitly fetching the URI with thefetch
query, or if the entry is a file (stat
is called for files open in the editor).Considering this causes a balloon in API calls when listing data sets and USS folders, it would be best to get this fix into the upcoming pre-release, or at the very least get the fix merged before v3 GA so that our users don't encounter this problem 😅
How to test:
NOTE: Please test on a dev LPAR as this may spam the mainframe with API calls
List a large data set in Zowe Explorer. In this PR it will load very quickly - in the last pre-release it will take quite some time to complete (or it won't finish loading at all)
Release Notes
Milestone: vNext
Changelog:
Types of changes
Checklist
General
yarn workspace vscode-extension-for-zowe vscode:prepublish
pnpm --filter vscode-extension-for-zowe vscode:prepublish
Code coverage
Deployment