[zos-jobs] Added support for cancel_job version number on requests
#106
Conversation
Signed-off-by: aadityasinha-dotcom <aadityasinha009@gmail.com>
aadityasinha-dotcom
force-pushed
the
cancel_jobs_version
branch
from
de0f2b6
to
973aff7
Compare
Codecov ReportBase: 73.70% // Head: 63.98% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## next #106 +/- ##
==========================================
- Coverage 73.70% 63.98% -9.72%
==========================================
Files 29 28 -1
Lines 1198 1008 -190
==========================================
- Hits 883 645 -238
- Misses 315 363 +48
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@aadityasinha-dotcom Thanks for your contribution 🙂 Please add unit and integration tests for the |
Signed-off-by: Aaditya Sinha <75474786+aadityasinha-dotcom@users.noreply.github.com>
There was a problem hiding this comment.
Thanks for working on this.
I left some comments that could improve the overall usage of the API 😋
One additional consideration.
There are some z/OSMF REST APIs that don't return all headers if the response is empty.
For that reason, maybe we should consider the following small change too:
From self.response.headers['Content-Type'] to self.response.headers.get('Content-Type') in
src/core/zowe/core_for_zowe_sdk/request_handler.py
(Thanks @t1m0thyj for helping me understand what was happening)
| def cancel_job(self, jobname: str, jobid: str, modify_version="2.0"): | ||
| def cancel_job(self, jobname, jobid, modifyVersion="1.0"): |
There was a problem hiding this comment.
will it be considered a breaking change if we change the modifyVersion default value ?
Maybe it should stay at 2.0 (synchronous by default) since there are other z/OS configuration considerations (e.g. CIM Authorization) for asynchronous requests.
| @@ -56,7 +56,7 @@ def get_job_status(self, jobname, jobid): | |||
| response_json = self.request_handler.perform_request("GET", custom_args) | |||
| return response_json | |||
|
|
|||
| def cancel_job(self, jobname: str, jobid: str, modify_version="2.0"): | |||
There was a problem hiding this comment.
I believe there is a missing modify_version to be renamed to modifyVersion
I don't know what the convention is, but it seems that this file is mostly using snake_case instead of camelCase
Can we rename it back to modify_version ?
| custom_args["json"] = {"request": "cancel", "version": modifyVersion} | ||
| response_json = self.request_handler.perform_request("PUT", custom_args, expected_code = [202]) | ||
| custom_args["json"] = {"request": "cancel"} | ||
| response_json = self.request_handler.perform_request("PUT", custom_args, expected_code=[202]) |
There was a problem hiding this comment.
Please remove the duplicate REST calls and use 202, 200 for the expected_code.
|
Looks like this enhancement was already made in #124. Sorry about that 😢 Going forward, we can avoid this happening again by making sure to assign issues to avoid duplicate work 🙂 |
No worries |
Added an optional
modifyVersionparameter to thecancel_jobAPI with a default value of"2.0"