-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
Ovirt add job #55440
Ovirt add job #55440
Conversation
) | ||
|
||
|
||
def build_job(module): |
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.
pass here just description, as it's the only parameter you need.
) | ||
|
||
|
||
def attach_steps(module, entity_id, jobs_service): |
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.
here you need just a 'steps' not whole module as well.
) | ||
|
||
|
||
def attach_steps(module, entity_id, jobs_service): |
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.
also please rename entity_id to job_id, no need to have it too generic here
ret = { | ||
'changed': changed, | ||
'id': getattr(job, 'id', None), | ||
type(job).__name__.lower(): get_dict_of_struct( |
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.
change 'type(job).name.lower()' to just 'job' no need to do it generic here
type(job).__name__.lower(): get_dict_of_struct( | ||
struct=job, | ||
connection=connection, | ||
fetch_nested=module.params.get('fetch_nested'), |
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 would add here fetch_nested=True, because we always want to return steps.
required: true | ||
state: | ||
description: | ||
- "Should the setp be present/absent/failed." |
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.
s/setp/step
state: | ||
description: | ||
- "Should the job be present/absent/failed." | ||
- "Present have alias started and absent have alias finished, you can use both." |
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.
please use C() around possible values of the state
state: | ||
description: | ||
- "Should the job be present/absent/failed." | ||
- "Present have alias started and absent have alias finished, you can use both." |
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 would write: C(started) is alias for C(present). C(finished) is alias for C(absent). Same in the steps.
@KKoukiou @derez @machacekondra @mwperina @nasx As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
shipit |
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.
shipit
shipit |
* init ovirt_job module * ovirt job add state finished * redone logic in ovirt job * ovirt jobs add docs * ovirt job update docs * update failed status * ovirt job update return docs * ovirt job update docs * check if job and step has finished * remove job status end
SUMMARY
Ovirt add job module
ISSUE TYPE
COMPONENT NAME
Ovirt
ADDITIONAL INFORMATION