-
Notifications
You must be signed in to change notification settings - Fork 61
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
Search task added #177
Search task added #177
Conversation
Code of search task and search model will be the same. Only url differs. Maybe should I add boolean parameter to function to check whether user searching model or task. In example: search(model=True, json="...") or search(task=True, json="..."). Or is it inconvenient to customers? |
Good point. But for simplification, may be just keep two separate function. We can revise later if we need to. |
Ok |
Codecov Report
@@ Coverage Diff @@
## main #177 +/- ##
==========================================
+ Coverage 90.79% 90.86% +0.06%
==========================================
Files 37 37
Lines 4042 4071 +29
==========================================
+ Hits 3670 3699 +29
Misses 372 372
|
elif isinstance(json_input, dict): | ||
API_BODY = json.dumps(json_input) | ||
else: | ||
return "Invalid JSON object passed as argument." |
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.
Can we cover this line with testing? May be we can write separate test function just to cover this line.
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 mean "else" block? I covered both dict and string test.
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.
"return "Invalid JSON object passed as argument." this block
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.
Ah I see maybe you mean try catch block missed for dict case. I will add it.
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.
"return "Invalid JSON object passed as argument." this block
Ok
Function accepts json string or in dict format. Test changes are added to check whether function will accept string and dict as input request body parameters. Signed-off-by: Nurlan <nabzalbekov0@gmail.com>
Added "else" block test case. Signed-off-by: Nurlan <nabzalbekov0@gmail.com>
@@ -372,6 +372,26 @@ def test_integration_model_train_register_full_cycle(): | |||
print("Model Task Status:", ml_task_status) | |||
raised = True | |||
assert raised == False, "Raised Exception in pulling task info" | |||
|
|||
try: | |||
search_task_obj = ml_client.search_task(json="{'_id':'{task_id}'}") |
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 saw parameter name was: json_input
. Then how can we invoke with json
?
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.
Yes, my bad sorry. I will change it.
try: | ||
search_task_obj = ml_client.search_task(json=15) | ||
assert search_task_obj == "Invalid JSON object passed as argument." | ||
except: # noqa: E722 | ||
raised = True | ||
assert raised == False, "Raised Exception in searching 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.
can we make a separate function for this to test? This chain of flow test was to test integration test. We don't need to include this test here.
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.
can we make a separate function for this to test? This chain of flow test was to test integration test. We don't need to include this test here.
Ok.
Signed-off-by: Nurlan <nabzalbekov0@gmail.com>
Signed-off-by: Nurlan <nabzalbekov0@gmail.com>
json_obj = json.loads(input_json) | ||
API_BODY = json.dumps(json_obj) | ||
except json.JSONDecodeError: | ||
return "Invalid JSON string passed as argument." |
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.
Can we cover this section also? We can easily cover by sending ml_client.search_model(input_json='15')
tests/common.py
Outdated
@@ -57,7 +57,7 @@ | |||
) | |||
_pd_ecommerce.insert(2, "customer_birth_date", None) | |||
_pd_ecommerce.index = _pd_ecommerce.index.map(str) # make index 'object' not int | |||
_pd_ecommerce["customer_birth_date"].astype("datetime64") | |||
_pd_ecommerce["customer_birth_date"].astype("datetime64[ns]") |
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.
Why did we need to change this?
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.
Because when running pytest this specific line throws error. Error suggests that I should replace datetime with datetime[ns]
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.
Are you not encountered with this error?
Signed-off-by: Nurlan <nabzalbekov0@gmail.com>
@@ -100,6 +100,102 @@ def test_execute(): | |||
), "Raised Exception during execute API testing with JSON string" | |||
|
|||
|
|||
def test_search(): |
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.
Will it be better if we print out error message?
* Search task added Function accepts json string or in dict format. Test changes are added to check whether function will accept string and dict as input request body parameters. Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Another test case added Added "else" block test case. Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Test corrected Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Search model added and code was corrected little bit Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Linting Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Additional test case added for each search api: task and model Signed-off-by: Nurlan <nabzalbekov0@gmail.com> --------- Signed-off-by: Nurlan <nabzalbekov0@gmail.com> (cherry picked from commit 57f0cf8)
* Search task added Function accepts json string or in dict format. Test changes are added to check whether function will accept string and dict as input request body parameters. Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Another test case added Added "else" block test case. Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Test corrected Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Search model added and code was corrected little bit Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Linting Signed-off-by: Nurlan <nabzalbekov0@gmail.com> * Additional test case added for each search api: task and model Signed-off-by: Nurlan <nabzalbekov0@gmail.com> --------- Signed-off-by: Nurlan <nabzalbekov0@gmail.com> (cherry picked from commit 57f0cf8) Co-authored-by: Nurlan <nabzalbekov0@gmail.com>
Function accepts json string or in dict format. Test changes are added to check whether function will accept string and dict as input request body parameters.
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.