- 
                Notifications
    You must be signed in to change notification settings 
- Fork 92
Fix overwrite option not being used in data set and uss file downloads #2620
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
        
          
                packages/zosfiles/__tests__/__unit__/methods/download/Download.unit.test.ts
              
                Fixed
          
            Show fixed
            Hide fixed
        
      Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
| Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@            Coverage Diff             @@
##           master    #2620      +/-   ##
==========================================
+ Coverage   91.81%   91.83%   +0.01%     
==========================================
  Files         644      644              
  Lines       19143    19181      +38     
  Branches     4124     4218      +94     
==========================================
+ Hits        17577    17615      +38     
  Misses       1564     1564              
  Partials        2        2              ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
Signed-off-by: JWaters02 <[email protected]>
| I suppose there should probably be some system tests but vast majority of existing download system tests fail for me probably due to some misconfiguration but idk if Ross would even be very happy to have a massive number of requests come on the zxplore system. | 
| Regarding the failed unit tests, I only see one failing test on Windows but 2 failing tests on other platforms: The test  As for the second test, this seems like an issue specific to building the correct destination path on Linux and MacOS. This may be related to path normalization between platforms - you might want to double check that the platform-specific path import is used to ensure proper path generation, e.g.  Regarding the failed integration tests, these are all due to snapshots that have updated from the change. If you run  | 
| 
 Hey sorry I know I must've asked/figured this out before, but which folder do I run this in? in root folder, I get error and then in packages/zosfiles there is no integration test script to run | 
| 
 You should be able to run from the root, it seems that you're encountering an issue when installing the test/sample CLIs as part of the pretest script - if you run  | 
| 
 Nope | 
| @JWaters02 revisiting the error details, I think the issue may be a side effect of having a space in the path of your current working directory ("GitHub Projects"). Can you try renaming that folder or moving the repo to a path without a space to see if the issue goes away? | 
| 
 As yes good observation, you are right. I fixed this in #2627 | 
Signed-off-by: JWaters02 <[email protected]>
| @t1m0thyj Hi is this a misclick? Any chance you could re-open the PR? | 
| @JWaters02 Apologies for this, it should be re-opened now. There wasn't a misclick - when a PR is merged, GitHub auto-closes issues/PRs that are mentioned with the "Fixes" or "Resolves" keyword. So the description of #2627 triggered it: 
 | 
| 
 Ahh okay I see thanks. I knew it did it for issues but had never seen it do it for PRs 😅 | 
Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: Joshua Waters <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
Signed-off-by: JWaters02 <[email protected]>
| 
 @traeok not sure whats going wrong, mind taking another look again? I can't test on unix right now (I plan to set up a unix box for zowe on proxmox server just need the time to get round to it). apologies | 
| 
 Sorry for the delay - I'll take a look 👍 | 
Signed-off-by: Trae Yelovich <[email protected]>
| @JWaters02 Regarding issues with Mac/Linux pipelines: 
 Fixed in 46eb97d - the mock implementation for  | 
| 
 | 
| 
 Ooh okay I see now so just silly mistake. Was just getting confused then. Thanks so much as usual trae ❤️ | 
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, thanks Joshua for the fix!
|  | ||
| ## Recent Changes | ||
|  | ||
| - Added support for `--overwrite` option to all Download methods. The default behavior is no longer to always overwrite existing files. [#2620](https://github.com/zowe/zowe-cli/pull/2620) | 
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.
(nitpick) missing BugFix prefix here



What It Does
Fixes #2594
Added new interfaces
IDownloadAmResultandIDownloadAmResponseto provide detailed results when downloading all members of a PDS.Added new optional field
skippedExistingtoIDownloadDsmResult.How to Test
Run zowe zos-files download all-members, all-members-matching, data-set, data-sets-matching and uss-file commands with and without overwrite option, deleting some files in the middle to test capabilities of each.
Review Checklist
I certify that I have:
Additional Comments
There is one failing test, but I am stumped how to fix it...