-
Notifications
You must be signed in to change notification settings - Fork 45
[New module] zos_started_task #2275
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: dev
Are you sure you want to change the base?
Conversation
* add nonscratch parameter in main module * added support for nonscratch in module_utils * added test case * added fragments * FIXED * changed in fragments * Updated changelogs * reviewed comments * fixed * removed print statement * removed extra validation --------- Co-authored-by: Fernando Flores <[email protected]>
…ible-collections/ibm_zos_core into enhancement/zos_started_task
…d_task Merging latest dev changes
Display command response:
|
Start command response with verbose:
|
Cancel command response with verbose:
|
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.
Requested few changes, only got to some of the docs will continue later
return contents | ||
|
||
def _basic_dict_type(self, contents, resolve_dependencies): | ||
"""Resolver for str type arguments. |
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.
"""Resolver for str type arguments. | |
"""Resolver for basic dict type arguments. |
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.
Done
return str(contents) | ||
|
||
def _identifier_name_type(self, contents, resolve_dependencies): | ||
"""Resolver for data_set type arguments. |
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.
Since this is used only in zos_started task could you comment a bit of where this is used ?
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.
Added
return contents | ||
|
||
def _member_name_type(self, contents, resolve_dependencies): | ||
"""Resolver for data_set type arguments. |
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.
Since this is used only in zos_started task could you comment a bit of where and how this is used ? I mean, add as a comment in the function, so that is not used for other reasons. Is this member a data set member?
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.
Updated. Member is the PDS/E member.
plugins/modules/zos_started_task.py
Outdated
state: | ||
description: | ||
- The desired state the started task should be after the module is executed. | ||
- If state=started and the started task is not found on the managed node, no action is taken, |
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.
I think this behavior should be changed, just checked the ansible.services module and when a service is not found in the managed node it fails
fatal: [zvm]: FAILED! => {"ansible_facts": {"discovered_interpreter_python": "/usr/bin/python3.12"}, "changed": false, "msg": "Could not find the requested service hpptt: host"}
This seems more aligned to the idea of idempotency, returning a succesful response with changed: false would give a false sense that the task is running already, even though we can start more than one.
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.
Modified description. Here status of the started task or member are validated by ZOAU.
plugins/modules/zos_started_task.py
Outdated
managed node, no action is taken, module completes successfully with changed=False. | ||
- If state is modified and the started task is not running, not found or modification was not | ||
done, the module will fail. | ||
- If state is displayed the module will return the started task details. |
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.
In the case of state being displayed what is the changed return value? could you add that?
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.
Added
plugins/modules/zos_started_task.py
Outdated
default: false | ||
wait_time: | ||
description: | ||
- Option wait_time is the total time that module zos_started_tak will wait for a submitted |
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.
- Option wait_time is the total time that module zos_started_tak will wait for a submitted | |
- Option wait_time is the total time that module zos_started_task will wait for a submitted |
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.
corrected
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.
I'll wait for you to address fernando's comments on return process from zos_started_task.
…ible-collections/ibm_zos_core into enhancement/zos_started_task
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.
Good work you have done in this module. There's a lot of comments, but they're mostly documentation or reminders of how to do things more in Python's style. Another question I have about cpu_time
and elapsed_time
: the time returned changes depending on how long the started task has been running for, would it be too cumbersome to convert all those formats into a single one like this: hhhhh.mm.ss.ttt? I'm thinking having the same format all the time would make automation easier. Of course, special values ********
and NOTAVAIL
would remain the same.
All comments I've made are probably a lot of work and you've been waiting on more reviews, so if I don't review this PR again quickly please ping me
@rexemin Thank you for the review.. Other comments are addressed. |
…ible-collections/ibm_zos_core into enhancement/zos_started_task
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.
Surendra... I think you need to re-push your changes... the 'view changed' did not show changes that you marked as resolved. We need to see the actual changes in github before we can approve it.
@richp405 this comment applicable at 2 places.. i missed updating here. Now updated.. |
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.
There are a few document issues remaining in zos_started_task, but not deal busters in my opinion. A nice chunk of code.
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.
Looks pretty great, thanks for working on so many comments so quickly, I just have a couple of other corrections and some questions from last week pending
else: | ||
module.fail_json( | ||
rc=5, | ||
msg="job_name is missing which is mandatory to display started task details.", |
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.
You are stopping a started task here
else: | ||
module.fail_json( | ||
rc=5, | ||
msg="job_name is missing which is mandatory to display started task details.", |
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.
you are modifying a started task here
SUMMARY
New module zos_started_task which support start, display, modify, cancel, stop and force operations.
ISSUE TYPE
COMPONENT NAME
zos_started_task
ADDITIONAL INFORMATION