Enhancement, improve load module and program object support in zos_copy #804
Conversation
…ram_object_support
…ram_object_support
…_support' of https://github.com/ansible-collections/ibm_zos_core into enhancement/652/Improved_load_module_and_program_object_support
…ram_object_support
ddimatos
added
the
Do not Merge
|
This is a 1.8 item but if its in good shape we can release it in 1.7 but I would like to also be part of the review list. I labeled it with do not merge only because it needs some extra reviews since its a 1.8 item but great work on finishing early. |
…ram_object_support
…_support' of https://github.com/ansible-collections/ibm_zos_core into enhancement/652/Improved_load_module_and_program_object_support
plugins/modules/zos_copy.py
Outdated
| @@ -1490,21 +1520,12 @@ def copy_to_member( | |||
| if self.is_binary: | |||
| opts["options"] = "-B" | |||
|
|
|||
| if self.is_executable: | |||
| opts["options"] = "-X" | |||
There was a problem hiding this comment.
Noted as comment, Marcel and I discussed about the use of "I" option. This will keep aliases when copying, currently version 1.2.3 has some bugs with it, 1.2.4 appears to have fixed some of it.
There was a problem hiding this comment.
Agree I will add and run the pipeline again.
plugins/modules/zos_copy.py
Outdated
| else: | ||
| dest_params["record_format"] = "U" | ||
| dest_params["type"] = "LIBRARY" | ||
| data_set.DataSet.ensure_present(replace=force, **dest_params) |
There was a problem hiding this comment.
Can simplify to one if
plugins/modules/zos_copy.py
Outdated
| elif dest_ds_type in data_set.DataSet.MVS_SEQ: | ||
| volumes = [volume] if volume else None | ||
| data_set.DataSet.ensure_absent(dest, volumes=volumes) | ||
|
|
||
| if src_ds_type == "USS": | ||
| # Taking the temp file when a local file was copied with sftp. | ||
| create_seq_dataset_from_file(src, dest, force, is_binary, volume=volume) | ||
| create_seq_dataset_from_file(src, dest, force, is_binary, is_executable, volume=volume) |
There was a problem hiding this comment.
is_executable is not used in this function
|
New pipeline run with -IX as option of copy |
…ram_object_support
There was a problem hiding this comment.
Overall, SOLID work here @AndreMarcel99, this PR turned out to be much more complex than initially expected. A few generic comments:
-
Not a deal-breaker, but why was the parameter name changed from
is_executabletoexecutable? The original issue description and even the PR description refers tois_executable. It seems more consistent to me to useis_executable, similar tois_binary. Curious to know if there was a particular reason for removing theis_prefix. -
Would be good to include an example in the EXAMPLES section (feel free to change below example for more clarity/accuracy):
- name: Copy a Program Object on remote system to a new PDSE.
zos_copy:
src: HLQ.COBOLSRC.PDSE(TESTPGM)
dest: HLQ.NEW.PDSE
remote_src: true
executable: true
changelogs/fragments/804-improved_load_module_and_program_object_support.yml
Show resolved
Hide resolved
|
is_executable was the first version but the suggestion was to change it |
There was a problem hiding this comment.
All docs stuff here
- In the notes section of DOCUMENTATION - change
is_executabletoexecutable - In the example task - modify the example slightly.
- Edit
destoption description to include info about executable option. - From
executableoption description, remove irrelevant lines, add Demetri's suggested lines. - In module_utils/action/copy.py - update copyright years (2019-2023).
(2)
- name: Copy a Program Object on remote system to a new PDSE member MYCOBOL.
zos_copy:
src: HLQ.COBOLSRC.PDSE(TESTPGM)
dest: HLQ.NEW.PDSE(MYCOBOL)
remote_src: true
executable: true(3) The edit adds one sentence to the end of the existing line:
- If C(dest) is a nonexistent data set, the attributes assigned will depend on the type of
C(src). If C(src) is a USS file, C(dest) will have a Fixed Block (FB) record format and the
remaining attributes will be computed. If I(is_binary=true), C(dest) will have a Fixed Block
(FB) record format with a record length of 80, block size of 32760, and the remaining
attributes will be computed. If I(executable=true),C(dest) will have an Undefined (U) record
format with a record length of 0, block size of 32760, and the remaining attributes will be
computed.(4)
executable:
description:
- <REMOVE> If C(dest) is a nonexistent data set, the attributes assigned will depend
on the type of C(src). If C(src) is a USS file, C(dest) will have a
Fixed Block (FB) record format and the remaining attributes will be computed.
- <REMOVE> If I(executable=true),C(dest) will have a Undefined (U)record format
with a record length of 0, block size of 32760, and the remaining
attributes will be computed.
- <REMOVE> When C(dest) is a data set, precedence rules apply. If C(dest_data_set)
is set, this will take precedence over an existing data set. If C(dest)
is an empty data set, the empty data set will be written with the
expectation its attributes satisfy the copy.
- <ADD> If set to C(true), indicates that the file or library to be copied is an executable.
- <ADD> If the C(src) executable has an alias, the alias information is also copied. If the
C(dest) is Unix, the alias is not visible in Unix, even though the information is there and
will be visible if copied to a library.
- <ADD> If I(executable=true), and C(dest) is a data set, it must be a PDS or PDSE (library).
- <ADD> If C(dest) is a nonexistent data set, the library attributes assigned will be
Undefined (U) record format with a record length of 0, block size of 32760 and the
remaining attributes will be computed.
- If C(dest) is a file, execute permission for the user will be added to the file (``u+x``).
|
ketankelkar
added
Ready to Merge
…ram_object_support
|
fernandofloresg
dismissed
ddimatos’s stale review
Demetri's changes requested were addressed by Ketan and Marcel. D is on vacations hence dismiss the review to be able to merge.
fernandofloresg
deleted the
enhancement/652/Improved_load_module_and_program_object_support
branch
SUMMARY
To improve load of executable files from uss file or dataset to any dataset or uss file also cover the variables in cat to change permissions and adjust how the module create a respective library to execute the file in case the dataset of target don't exists also cover files to add execution to the owner and in case of directory cover the permissions by setting the execution permissions with or to all files if the user set is_excutable to true.
Fixing #652
ISSUE TYPE
COMPONENT NAME
Add is_executable option to the user set true or false.
Pass throw the validations inside the module check if the user give information of the creation of target add or ensure the record_format and type ensure the target the creation of an executable or modify permissions on the file.
IMPORTANT NOTES
The enhanced could had problems with ZOAU 1.2.3 but with latest (1.2.4) works fine.
ADDITIONAL INFORMATION
Cover cases:
is_executable_files_and_playbook.zip