Search task added #177

Nurlanprog · 2023-06-14T18:15:59Z

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

New functionality includes testing.
- All tests pass
New functionality has been documented.
Commits are signed per the DCO using --signoff

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.

Nurlanprog · 2023-06-14T18:24:48Z

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?

dhrubo-os · 2023-06-14T18:32:17Z

Good point. But for simplification, may be just keep two separate function. We can revise later if we need to.

Nurlanprog · 2023-06-14T18:33:51Z

Ok

codecov · 2023-06-14T18:42:05Z

Codecov Report

Merging #177 (867e708) into main (8eb26eb) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            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

Impacted Files	Coverage Δ
opensearch_py_ml/ml_commons/ml_commons_client.py	`83.76% <100.00%> (+3.76%)`	⬆️

dhrubo-os · 2023-06-14T18:48:19Z

opensearch_py_ml/ml_commons/ml_commons_client.py

+        elif isinstance(json_input, dict):
+            API_BODY = json.dumps(json_input)
+        else:
+            return "Invalid JSON object passed as argument."


Can we cover this line with testing? May be we can write separate test function just to cover this line.

You mean "else" block? I covered both dict and string test.

"return "Invalid JSON object passed as argument." this block

Ah I see maybe you mean try catch block missed for dict case. I will add it.

"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>

dhrubo-os · 2023-06-15T01:04:54Z

tests/ml_commons/test_ml_commons_client.py

@@ -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}'}")


I saw parameter name was: json_input. Then how can we invoke with json?

Yes, my bad sorry. I will change it.

dhrubo-os · 2023-06-15T01:06:28Z

tests/ml_commons/test_ml_commons_client.py

+                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"


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.

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>

dhrubo-os · 2023-06-15T14:40:01Z

opensearch_py_ml/ml_commons/ml_commons_client.py

+                json_obj = json.loads(input_json)
+                API_BODY = json.dumps(json_obj)
+            except json.JSONDecodeError:
+                return "Invalid JSON string passed as argument."


Can we cover this section also? We can easily cover by sending ml_client.search_model(input_json='15')

dhrubo-os · 2023-06-15T15:13:26Z

tests/common.py

@@ -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]")


Why did we need to change this?

Because when running pytest this specific line throws error. Error suggests that I should replace datetime with datetime[ns]

Are you not encountered with this error?

Signed-off-by: Nurlan <nabzalbekov0@gmail.com>

b4sjoo · 2023-06-19T18:35:01Z

tests/ml_commons/test_ml_commons_client.py

@@ -100,6 +100,102 @@ def test_execute():
    ), "Raised Exception during execute API testing with JSON string"


+def test_search():


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>

Nurlanprog requested review from dhrubo-os, greaa-aws and ylwu-amzn as code owners June 14, 2023 18:16

dhrubo-os reviewed Jun 14, 2023

View reviewed changes

Nurlanprog added 2 commits June 15, 2023 06:33

Search task added

4c8fbf9

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

d995566

Added "else" block test case. Signed-off-by: Nurlan <nabzalbekov0@gmail.com>

dhrubo-os reviewed Jun 15, 2023

View reviewed changes

Nurlanprog added 3 commits June 15, 2023 16:05

Test corrected

454499d

Signed-off-by: Nurlan <nabzalbekov0@gmail.com>

Search model added and code was corrected little bit

fa5ccbb

Signed-off-by: Nurlan <nabzalbekov0@gmail.com>

Linting

33be035

Signed-off-by: Nurlan <nabzalbekov0@gmail.com>

dhrubo-os reviewed Jun 15, 2023

View reviewed changes

Additional test case added for each search api: task and model

867e708

Signed-off-by: Nurlan <nabzalbekov0@gmail.com>

dhrubo-os approved these changes Jun 16, 2023

View reviewed changes

b4sjoo reviewed Jun 19, 2023

View reviewed changes

rbhavna approved these changes Jul 20, 2023

View reviewed changes

dhrubo-os merged commit 57f0cf8 into opensearch-project:main Jul 20, 2023

dhrubo-os added the backport 1.x label Jul 20, 2023

opensearch-trigger-bot bot mentioned this pull request Jul 20, 2023

[Backport 1.x] Search task added #197

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search task added #177

Search task added #177

Nurlanprog commented Jun 14, 2023

Nurlanprog commented Jun 14, 2023

dhrubo-os commented Jun 14, 2023

Nurlanprog commented Jun 14, 2023

codecov bot commented Jun 14, 2023 •

edited

Loading

dhrubo-os Jun 14, 2023 •

edited

Loading

Nurlanprog Jun 14, 2023

dhrubo-os Jun 14, 2023

Nurlanprog Jun 14, 2023

Nurlanprog Jun 14, 2023

dhrubo-os Jun 15, 2023

Nurlanprog Jun 15, 2023

dhrubo-os Jun 15, 2023

Nurlanprog Jun 15, 2023

dhrubo-os Jun 15, 2023

dhrubo-os Jun 15, 2023

Nurlanprog Jun 15, 2023

Nurlanprog Jun 15, 2023

b4sjoo Jun 19, 2023

		@@ -100,6 +100,102 @@ def test_execute():
		), "Raised Exception during execute API testing with JSON string"


		def test_search():

Search task added #177

Search task added #177

Conversation

Nurlanprog commented Jun 14, 2023

Issues Resolved

Check List

Nurlanprog commented Jun 14, 2023

dhrubo-os commented Jun 14, 2023

Nurlanprog commented Jun 14, 2023

codecov bot commented Jun 14, 2023 • edited Loading

Codecov Report

dhrubo-os Jun 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 14, 2023 •

edited

Loading

dhrubo-os Jun 14, 2023 •

edited

Loading