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

Enhancement/v1.4.0/zos copy dirs #561

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

rexemin
Copy link
Collaborator

@rexemin rexemin commented Nov 10, 2022

SUMMARY

Fixes #553 and #554.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

zos_copy

ADDITIONAL INFORMATION

For #553: added a test case for the creation of missing parents.
For #554: the module now only gets the original permissions for files and subdirectories that will be overwritten, instead of walking the whole directory tree for the destination.

Additionally, when the source is a file and the destination is a directory, the module computes the final destination path before trying to backup. Before this change, it tried to backup the whole destination directory, when it only needed to backup one file.

Copy link
Collaborator

@richp405 richp405 left a comment

Choose a reason for hiding this comment

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

Looks okay so far... just wanted you to know I looked through it. Let me know when you're ready for formal review - richp

@rexemin rexemin marked this pull request as ready for review November 10, 2022 21:36
@rexemin
Copy link
Collaborator Author

rexemin commented Nov 10, 2022

Thanks for the preliminary review @richp405. I think this PR is ready for a formal review, especially around the test cases. I didn't add new ones for #554, as that is somewhat tested already by test_copy_local_dir_and_change_mode, but if you have any suggestions for adding more test coverage please let me know.

Copy link
Collaborator

@richp405 richp405 left a comment

Choose a reason for hiding this comment

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

looks fine

@rexemin
Copy link
Collaborator Author

rexemin commented Nov 14, 2022

Ran the Jenkins pipeline on the current state of this branch.

Screen Shot 2022-11-14 at 11 58 14
Screen Shot 2022-11-14 at 11 58 37

Copy link
Collaborator

@fernandofloresg fernandofloresg left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@ketankelkar ketankelkar left a comment

Choose a reason for hiding this comment

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

lgtm

@fernandofloresg fernandofloresg merged commit 8c87f28 into support/1.4.0 Nov 15, 2022
@ddimatos ddimatos deleted the Enhancement/v1.4.0/zos-copy-dirs branch April 15, 2023 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants