Skip to content

Use target_index_body for engine detection instead of radial_engine - #1099

Merged
OVI3D0 merged 1 commit into
opensearch-project:mainfrom
Likhoram:fix/radial-engine-detection
Jul 31, 2026
Merged

Use target_index_body for engine detection instead of radial_engine#1099
OVI3D0 merged 1 commit into
opensearch-project:mainfrom
Likhoram:fix/radial-engine-detection

Conversation

@Likhoram

Copy link
Copy Markdown
Contributor

Description

Use target_index_body for engine detection instead of radial_engine param.

Testing

Validated via unit tests. Template passthrough follows same pattern as radial_search_type which is already working in production.


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.

…aram

Signed-off-by: Wenxin Li <liwenxin@amazon.com>
Copilot AI review requested due to automatic review settings July 27, 2026 21:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Possible Issue

target_index_body is expected to be a file path (e.g., indices/faiss-index.json), but the detection logic only checks whether the substring lucene appears in that path string. If a user supplies a path that does not contain the engine name (e.g., indices/my-index.json for a Lucene index, or a custom filename), the engine will silently default to faiss, potentially running the benchmark against the wrong engine. Additionally, if target_index_body is not a string (e.g., None or a dict), the .lower() call will raise an AttributeError. Consider parsing the index body file contents to detect the engine, or validating the input type.

index_body = params.get("target_index_body", "")
if "lucene" in index_body.lower():
    self.radial_engine = "lucene"
else:
    self.radial_engine = "faiss"

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Guard against non-string index body values

params.get("target_index_body", "") can return a non-string value (e.g., None or a
dict) if the parameter is set to a non-default type, causing AttributeError on
.lower(). Coerce to string before calling .lower() to make the detection robust.

osbenchmark/workload/params.py [1167-1171]

 if self.radial_search_type:
-    index_body = params.get("target_index_body", "")
+    index_body = str(params.get("target_index_body") or "")
     if "lucene" in index_body.lower():
         self.radial_engine = "lucene"
     else:
         self.radial_engine = "faiss"
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies a potential AttributeError if target_index_body is None or a non-string type. However, in typical usage target_index_body is a file path string, so the practical impact is minor defensive coding.

Low

@OVI3D0
OVI3D0 merged commit d1aea17 into opensearch-project:main Jul 31, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants