Skip to content

refactor: refactor copybook download #2295

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
Merged
merged 3 commits into from
May 23, 2024
Merged

refactor: refactor copybook download #2295

merged 3 commits into from
May 23, 2024

Conversation

ap891843
Copy link
Contributor

@ap891843 ap891843 commented Apr 26, 2024 •
edited
Loading

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • No side effects

Checklist:

  • Each of my commits contains one meaningful change
  • I have performed rebase of my branch on top of the development
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

@ap891843 ap891843 force-pushed the refactor/copybook_download branch from 44fcae7 to 07f10d0 Compare April 30, 2024 12:17
@ap891843 ap891843 changed the title [WIP] refactor: refactor copybook download refactor: refactor copybook download Apr 30, 2024
@ap891843 ap891843 marked this pull request as ready for review April 30, 2024 12:18
@ap891843 ap891843 changed the title refactor: refactor copybook download [Do not Merge] refactor: refactor copybook download Apr 30, 2024
@ap891843 ap891843 changed the title [Do not Merge] refactor: refactor copybook download [WIP] refactor: refactor copybook download Apr 30, 2024
@ap891843 ap891843 marked this pull request as draft May 6, 2024 08:57
@ap891843 ap891843 force-pushed the refactor/copybook_download branch from 07f10d0 to 7652b58 Compare May 7, 2024 12:25
@ap891843
Copy link
Contributor Author

ap891843 commented May 7, 2024 •
edited
Loading

Plan/ Discussed to merge this PR after #2265
Drafting this PR till #2265 is merged

@ap891843 ap891843 changed the title [WIP] refactor: refactor copybook download refactor: refactor copybook download May 7, 2024
@ap891843 ap891843 marked this pull request as ready for review May 7, 2024 14:18
@ap891843 ap891843 marked this pull request as draft May 7, 2024 14:19
@ap891843 ap891843 force-pushed the refactor/copybook_download branch from 7652b58 to fbf99b4 Compare May 20, 2024 09:26
@ap891843 ap891843 marked this pull request as ready for review May 20, 2024 09:26
@ap891843 ap891843 requested a review from slavek-kucera May 20, 2024 09:26
slavek-kucera
slavek-kucera reviewed May 20, 2024
View reviewed changes
clients/cobol-lsp-vscode-extension/src/services/copybook/downloader/ZoweExplorerDownloader.ts Outdated
}

protected handleError(e: any, profileName: string) {
if (this.isInvalidCredentials(e)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the async nature of the caller, I don't think this really solves the account lock-out in the scenario of many concurrent requests like after re-opening a workspace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still need to figure this out. will change this in next commit

Copy link
Contributor Author

@ap891843 ap891843 May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

updated at 6f18d26
Concurrency level is reduced but the issue remains

@ap891843 ap891843 requested a review from slavek-kucera May 21, 2024 10:03
@ap891843 ap891843 force-pushed the refactor/copybook_download branch 2 times, most recently from 63a5506 to 772f6c4 Compare May 21, 2024 16:09
@ap891843 ap891843 force-pushed the refactor/copybook_download branch from cd9304a to 6f18d26 Compare May 22, 2024 11:52
@ap891843 ap891843 requested a review from slavek-kucera May 22, 2024 11:53
@ap891843 ap891843 force-pushed the refactor/copybook_download branch from fd76850 to 4e83626 Compare May 22, 2024 12:36
@slavek-kucera
Copy link
Contributor

what is clients/cobol-lsp-vscode-extension/src/__tests__/services/copybook/DownloadQueueTest.specRemove.tsRmv? it looks like accidental rename.

slavek-kucera
slavek-kucera reviewed May 23, 2024
View reviewed changes
clients/cobol-lsp-vscode-extension/src/services/copybook/downloader/DownloadUtil.ts

try {
const profile = this.loadProfile(profileName, explorerAPI);
await explorerAPI.getUssApi(profile).fileList("/");
Copy link
Contributor

Choose a reason for hiding this comment

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

Does zowe API require the use to have access to USS? Is it possible to list/download datasets via zowe without the user having OMVS profile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am under the impression that the profiles are same and shared across MVS and USS. Added a random USS call to check for the credentials validity.

Copy link
Contributor

Choose a reason for hiding this comment

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

for the record: this is about OMVS segment in the user profile defined in the mainframe security product.

@ap891843 ap891843 force-pushed the refactor/copybook_download branch from 4e83626 to ef76609 Compare May 23, 2024 07:55
slavek-kucera
slavek-kucera approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@slavek-kucera slavek-kucera left a comment

Choose a reason for hiding this comment

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

pending testing.

@ap891843 ap891843 force-pushed the refactor/copybook_download branch from ef76609 to 233b635 Compare May 23, 2024 10:39
@ap891843 ap891843 merged commit 715abe6 into eclipse-che4z:development May 23, 2024
16 checks passed
ap891843 added a commit that referenced this pull request Mar 14, 2025
@ap891843 @slavek-kucera
refactor: refactor copybook download (#2295)
b4b6969 
* refactor: refactor copybook download


Signed-off-by: Aman Prashant <[email protected]>
Co-authored-by: slavek-kucera <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavek-kucera slavek-kucera slavek-kucera approved these changes

@ishche ishche ishche approved these changes

@VitGottwald VitGottwald Awaiting requested review from VitGottwald

@Nurkambay Nurkambay Awaiting requested review from Nurkambay

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ap891843 @slavek-kucera @ishche