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

Debounce credential prompts and check profile locks in more places #3480

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

traeok
Copy link
Member

@traeok traeok commented Feb 24, 2025

Proposed changes

  • Adds a 30 second timeout to waitForUnlock to prevent deadlocks when using the function (safety mechanism)
  • "Debounces" credential prompts from parallel requests to a mainframe system by only showing one credential prompt, until the user answers that prompt.
    • Behavior remains unchanged: If the user selects "Cancel" or closes the dialog, the profile remains locked. The user can update their profile through the tree views through the "Manage Profile" to unlock the profile in the filesystem.
    • Resolves an issue where the user was spammed with multiple credential prompts when using a virtual workspace with a profile w/ invalid credentials.
  • Eliminated cached profile use in filesystems: All filesystem requests will get the latest profile object to prevent invalid, cached credentials from being used.
    • The issue still exists in tree views for credential desync in the quick picks. I have filed issue # to track/resolve this in the future.

Release Notes

Milestone: 3.1.2

Changelog:

  • Fixed issue where users were prompted several times when using a profile with invalid credentials in a VS Code workspace. Now, the user is only prompted once per profile, allowing the user to enter in new credentials.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Breaking change (a change that would cause existing functionality to not work as expected)
  • Documentation (Markdown, README updates)
  • Other (please specify above in "Proposed changes" section)

Checklist

General

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • All PR dependencies have been merged and published (if applicable)
  • A GIF or screenshot is included in the PR for visual changes
  • The pre-publish command has been executed:
    • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
    • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

  • There is coverage for the code that I have added
  • I have added new test cases and they are passing
  • I have manually tested the changes

Deployment

  • I have added developer documentation (if applicable)
  • Documentation should be added to Zowe Docs (discussing w/ Ana offline)
    • If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
    • Otherwise, please check with the rest of the squad about any needed documentation before merging.
  • These changes may need ported to the appropriate branches (list here):

Copy link

codecov bot commented Feb 24, 2025

Codecov Report

Attention: Patch coverage is 85.62092% with 22 lines in your changes missing coverage. Please review.

Project coverage is 93.38%. Comparing base (ce106c9) to head (adcc1cc).

Files with missing lines Patch % Lines
...kages/zowe-explorer/src/trees/uss/UssFSProvider.ts 84.88% 13 Missing ⚠️
...we-explorer/src/trees/dataset/DatasetFSProvider.ts 86.11% 5 Missing ⚠️
...ages/zowe-explorer-api/src/profiles/AuthHandler.ts 88.46% 3 Missing ⚠️
packages/zowe-explorer/src/utils/AuthUtils.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3480      +/-   ##
==========================================
- Coverage   93.49%   93.38%   -0.11%     
==========================================
  Files         120      120              
  Lines       12692    12800     +108     
  Branches     2767     2830      +63     
==========================================
+ Hits        11866    11953      +87     
- Misses        825      846      +21     
  Partials        1        1              

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

@traeok traeok changed the title refactor: Use locks in FS remote lookup to avoid credential loops in virtual workspaces Use locks in FS remote lookup to avoid credential loops in virtual workspaces Feb 24, 2025
@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 24, 2025
….waitForUnlock

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok force-pushed the fix/cred-loop-virtual-workspace branch from 510342d to f693249 Compare February 25, 2025 01:45
@pull-request-size pull-request-size bot added size/M and removed size/L labels Feb 25, 2025
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 25, 2025
… bugfix

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok changed the title Use locks in FS remote lookup to avoid credential loops in virtual workspaces Use locks in FS Feb 25, 2025
@traeok traeok changed the title Use locks in FS Debounce credential prompts and use locks in more places Feb 25, 2025
@traeok traeok changed the title Debounce credential prompts and use locks in more places Debounce credential prompts and check profile locks in more places Feb 25, 2025
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok force-pushed the fix/cred-loop-virtual-workspace branch from 00bfe7d to 9da0610 Compare February 25, 2025 20:47
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok added the no-changelog Add to PR's that don't require a CHANGELOG update label Feb 27, 2025
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok removed the no-changelog Add to PR's that don't require a CHANGELOG update label Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

1 participant