Created nd_backup_schedule module using Ansible Network Resource Modules Approach (DCNE-496) #169
Conversation
…ule jobs on ND 4.1 and later
… return the specific object
sajagana
force-pushed
the
nd_backup_schedule_new
branch
2 times, most recently
from
e7eae7a to
d018675
Compare
…k Resource Modules Approach
sajagana
force-pushed
the
nd_backup_schedule_new
branch
from
d018675 to
a6465d2
Compare
sajagana
changed the title
sajagana
requested review from
akinross,
anvitha-jain,
gmicol,
lhercot,
samiib and
shrsr
as code owners
| state = nd.params.get("state") | ||
| schedules = nd.request(base_path, method="GET").get("schedules") | ||
|
|
||
| remote_schedule_map = wrap_objects_by_key(schedules) |
There was a problem hiding this comment.
The assumption here is that you have a single identifier key, is this something that we would like to do?
|
|
||
| result = compare_config_and_remote_objects(schedules, config) | ||
|
|
||
| if state != "query": |
There was a problem hiding this comment.
this seems redundant because of the nd.exit_json() in line 209
| nd.after = schedules | ||
| nd.exit_json() | ||
|
|
||
| nd.before = copy.deepcopy(schedules) |
There was a problem hiding this comment.
should before and after not both display the same output for query states ?
|
|
||
| remote_schedule_map = wrap_objects_by_key(schedules) | ||
|
|
||
| if state == "query": |
There was a problem hiding this comment.
Are we supposed to expose query state? If I look at the doc you shared it seems fact modules should handle this, but perhaps I misunderstand it.
|
|
||
| nd.before = copy.deepcopy(schedules) | ||
|
|
||
| result = compare_config_and_remote_objects(schedules, config) |
There was a problem hiding this comment.
I find the naming of this function a bit weird because the order of your function is reversed of your input to the function.
| nd.after.append(payload) | ||
|
|
||
|
|
||
| def get_backup_schedule_time(scheduler_date, scheduler_time): |
There was a problem hiding this comment.
should this be a generic time handle utils function? likely we will need datetime formatting for other modules
| if not compare_unordered_list_of_dicts(nd.after, copy.deepcopy(nd.before)): | ||
| nd.result["changed"] = True |
There was a problem hiding this comment.
is this needed? cant we tell during a non GET request that is made already that there is a change?
| nd.exit_json() | ||
|
|
||
|
|
||
| def post_backup_schedule_config(nd, module, path, config_obj, remote_obj=None, method="POST"): |
There was a problem hiding this comment.
not sure the function name really makes sense when the method can also be PUT
|
|
||
| if state not in ["deleted", "absent"]: | ||
| for object in result.get("config_data_create"): | ||
| post_backup_schedule_config(nd, module, base_path, object) |
There was a problem hiding this comment.
did you investigate creating a generic request handler, and a module specific payload creator ( I assume that the end part of the function is repeatable for other modules )
| config = nd.params.get("config") | ||
| state = nd.params.get("state") | ||
| schedules = nd.request(base_path, method="GET").get("schedules") | ||
|
|
There was a problem hiding this comment.
Generic overall question, did you consider usage of class structure in python? this way the repeatable conditionals could be done from a single class method, which makes the module perhaps more readable.
No description provided.