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

Add --ignore-not-found flag logic to zosfilesBase.handler and update command definitions and en.ts for delete operations #2254

Merged
merged 47 commits into from
Nov 7, 2024

Conversation

ATorrise
Copy link
Contributor

@ATorrise ATorrise commented Sep 5, 2024

What it Does
This PR implements the --ignore-not-found flag for delete operations across DS, USS, zFS, and VSAM by:

  • Adding logic in the ZosFilesBaseHandler to handle --ignore-not-found and suppress errors when a file or data set does not exist (HTTP 404 or IDC3012I).
    • The IDC3012I code is an IBM z/OS error message that indicates a VSAM entry does not exist in the catalog
  • Updating the command definitions to include the --ignore-not-found flag.
  • Modifying en.ts to reflect the new flag in descriptions and examples.

image

How to Test
Verify that the --ignore-not-found flag works for all delete operations (DS, USS, zFS, VSAM) by attempting to delete things that don't exist. Delete non-existent files/data sets with and without --ignore-not-found.

  1. Test commands with --for-sure and --ignore-not-found flag (-fi)
Command Type
zowe zos-files delete data-set "ibmuser.cntl" -fi DS
zowe zos-files delete uss-file "/a/ibmuser/testcases" -fi USS
zowe zos-files delete zos-file-system "HLQ.MYNEW.ZFS" -fi zFS
zowe zos-files delete data-set-vsam "ibmuser.AAA.**.FFF" -fi VSAM
  1. Test commands with just the --for-sure flag (-f)
Command Type Expected Error Message
zowe zos-files delete data-set "ibmuser.cntl" -f DS "Data set not cataloged"
zowe zos-files delete uss-file "/a/ibmuser/testcases" -f USS "No such file or directory"
zowe zos-files delete zos-file-system "HLQ.MYNEW.ZFS" -f zFS "NOT FOUND"
zowe zos-files delete data-set-vsam "ibmuser.AAA.**.FFF" -f VSAM "NOT DELETED"

Review Checklist
I certify that I have:

  • I tested my changes.
  • I added/updated automated tests.
  • I updated the changelog.
  • I followed the contribution guidelines.

Additional Comments
This update is for all delete operations except MDS (which does not currently support this flag)

@ATorrise ATorrise linked an issue Sep 5, 2024 that may be closed by this pull request
Signed-off-by: ATorrise <[email protected]>
Copy link

codecov bot commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.16%. Comparing base (04219f6) to head (c61b943).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...kages/cli/src/zosfiles/delete/vsam/Vsam.handler.ts 90.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #2254    +/-   ##
========================================
  Coverage   91.16%   91.16%            
========================================
  Files         636      636            
  Lines       18034    18034            
  Branches     3770     3879   +109     
========================================
  Hits        16440    16440            
  Misses       1593     1593            
  Partials        1        1            

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

@ATorrise ATorrise changed the title Add --quiet flag logic to zosfilesBase.handler and update command definitions and en.ts for delete operations. Add --quiet flag logic to zosfilesBase.handler and update command definitions and en.ts for delete operations Sep 5, 2024
Signed-off-by: ATorrise <[email protected]>
@dkelosky
Copy link
Contributor

dkelosky commented Sep 6, 2024

Perhaps it's too late to adjust this, but I wanted to make an observation... --quite seems like a pretty generic flag that might have applicability to other command groups besides zowe files delete operations. Should it be reserved for a broader purpose?

I also came across this suggested "standards" reference which links to description:

‘quiet’
Used in many programs to inhibit the usual output. Every program accepting ‘--quiet’ should accept ‘--silent’ as a synonym.

Alternatively, a more descriptive command option for this command group could be used ~--ignore-not-found?

@zFernand0 zFernand0 self-requested a review September 9, 2024 20:56
@JTonda JTonda assigned zFernand0 and ATorrise and unassigned zFernand0 Sep 11, 2024
@ATorrise ATorrise marked this pull request as ready for review September 12, 2024 15:35
@ATorrise ATorrise requested review from t1m0thyj and awharn September 12, 2024 15:35
@ATorrise ATorrise marked this pull request as draft September 12, 2024 15:38
@t1m0thyj
Copy link
Member

Perhaps it's too late to adjust this, but I wanted to make an observation... --quiet seems like a pretty generic flag that might have applicability to other command groups besides zowe files delete operations. Should it be reserved for a broader purpose?

Alternatively, a more descriptive command option for this command group could be used ~--ignore-not-found?

I think this is a good point that --quiet or --silent are pretty generic and could be used for other purposes in the future such as controlling log level.

If it's not too late to change, I support the idea of --ignore-not-found which is more explicit and used by some other CLIs like kube-cli on their delete commands 😋

Signed-off-by: ATorrise <[email protected]>
@ATorrise ATorrise changed the title Add --quiet flag logic to zosfilesBase.handler and update command definitions and en.ts for delete operations Add --ignore-not-found flag logic to zosfilesBase.handler and update command definitions and en.ts for delete operations Sep 12, 2024
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋

@zFernand0 zFernand0 dismissed stale reviews from t1m0thyj and awharn November 5, 2024 20:16

I believe these requested changes were addressed in d20417c

Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

Looks pretty good, thanks @ATorrise! Left a few comments

@zFernand0
Copy link
Member

Added the no-chagelog label since the zosfiles sdk change was formatting the codebase

  • The changelog was not changed in this pull request for packages/zosfiles/CHANGELOG.md.

@zFernand0 zFernand0 requested a review from t1m0thyj November 6, 2024 16:08
@ATorrise
Copy link
Contributor Author

ATorrise commented Nov 6, 2024

Quality Gate Failed Quality Gate failed

Failed conditions 4.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@zFernand0 @t1m0thyj not sure what to do about this?

@zFernand0
Copy link
Member

Quality Gate Failed Quality Gate failed

Failed conditions 4.2% Duplication on New Code (required ≤ 3%)
See analysis details on SonarCloud

@zFernand0 @t1m0thyj not sure what to do about this?

I wouldn't worry too much about the duplication.
of course there could be some of it removed, but it might make the code a bit harder to follow.

If we do plan to reduce duplication, I guess I'd prefer to do it on a major release boundary, as part of our cleanup efforts 😋

Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @ATorrise for the new feature and thorough system tests!

packages/cli/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link

sonarqubecloud bot commented Nov 7, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@ATorrise ATorrise added the release-minor Indicates a minor feature has been added label Nov 7, 2024
@ATorrise ATorrise merged commit fdf1e70 into master Nov 7, 2024
18 of 19 checks passed
@ATorrise ATorrise deleted the quiet branch November 7, 2024 19:20
@github-actions github-actions bot removed the release-minor Indicates a minor feature has been added label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

How to make sure that a file is deleted with one command?
6 participants