Introduce lang-python module to enable Python as a scripting language in OpenSearch
#18095
Conversation
github-actions
bot
added
the
enhancement
yuancu
force-pushed
the
lang-python
branch
3 times, most recently
from
44668f9 to
952f477
Compare
|
❌ Gradle check result for 952f477: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
…n scripting Signed-off-by: Yuanchun Shen <[email protected]>
|
❌ Gradle check result for 248c987: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for f20a480: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Yuanchun Shen <[email protected]>
|
❌ Gradle check result for 70e4f2d: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
One thing I can think of is that infinite loop such as |
Thanks for reviewing! I am going to create a test for such cases. I'll let you know once I'm done. |
There was a problem hiding this comment.
Just take a first glance at the code, I think you can add more test cases and make sure all tests pass, and also make the code clean, good work! @yuancu
| buildscript { | ||
| ext { | ||
| opensearch_group = "org.opensearch" | ||
| opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT") |
There was a problem hiding this comment.
3.0.0 version is released, so snapshot is not needed for now.
| #!/bin/sh | ||
|
|
||
| # | ||
| # Copyright © 2015-2021 the original authors. |
There was a problem hiding this comment.
We should use the latest copyright header for new added files.
|
|
||
| private static FieldScript.LeafFactory newFieldScript( | ||
| String code, Map<String, Object> params, SearchLookup lookup) { | ||
| logger.info("Executing python code: {}", code); |
There was a problem hiding this comment.
Why not combining these 3 lines log into one line? And I think it should be debug level?
|
|
||
| // TODO: check whether code has a field `result` | ||
| context.eval("python", code); | ||
| logger.info("Eval succeeded"); |
There was a problem hiding this comment.
For a large scale cluster with high search load with python script is used, these kind of log would be enormous, we should downgrade the log level, I think.
| // } | ||
|
|
||
| Set<String> accessedDocFields = | ||
| PythonScriptUtility.extractAccessedDocFields(code); |
There was a problem hiding this comment.
Are the metadata fields like _id, _index, _routing can be accessed? If so do we have some use case? If we don't have some use cases then I think we should prevent the metadata fields from being accessed in the first version.
| @@ -0,0 +1,154 @@ | |||
| package org.opensearch.python.antlr; | |||
Description
This draft PR introduces the
lang-pythonmodule to OpenSearch.It is submitted for early code review and feedback on the implementation approach for enabling Python scripting support in OpenSearch.
Related Issues
Resolves #17432
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.