Skip to content
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

Feature/python inline task #275

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

inv-ajin
Copy link

@inv-ajin inv-ajin commented Oct 3, 2024

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

This PR implements support for Python as a scripting language for inline tasks in Netflix Conductor, based on the roadmap outlined here.
The feature extends Conductor's usability by enabling lightweight Python code execution as inline tasks.
The Python environment used for executing scripts is Jython.
Key changes include:

  • Added a new inline task type that supports executing Python scripts (previously only JavaScript was supported).
    
  • Introduced a new dependency for Jython in the build.gradle file inside conductor/core.
    
  • Implemented a new Evaluator to execute Python scripts located in conductor/core/java/core/executions/evaluators.
    
  • Added a new task type Python in the Java SDK.
    

Issue #

Alternatives considered

One alternative approach is to use GraalVM's Python engine instead of Jython, which could offer improved performance and compatibility.

Screenshot from 2024-10-03 15-04-33

Screenshot from 2024-10-03 15-02-08
Screenshot from 2024-10-03 15-01-14
Screenshot from 2024-10-03 15-00-14

Describe alternative implementation you have considered

Copy link
Collaborator

@v1r3n v1r3n left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I would suggest take a look at the GraalVM implementation that is supposed to offer better performance overall.

https://www.graalvm.org/python/

It will also be useful to add support have pre-defined set of libraries added to the engine, which will allow working with 3P libraries that are statically defined and can be added at runtime.

@@ -43,6 +43,9 @@ dependencies {

implementation "org.openjdk.nashorn:nashorn-core:15.4"

//jython dependency
implementation "org.python:jython-standalone:2.7.4"
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about using something like https://www.graalvm.org/python/

@inv-ajin
Copy link
Author

inv-ajin commented Oct 7, 2024

@v1r3n
Thank you for the feedback! We are planning to implement the Python inline task using GraalVM as suggested,
I'll keep you updated on the progress.

@inv-ajin
Copy link
Author

inv-ajin commented Oct 9, 2024

Hi,
@v1r3n I've made the requested changes and implemented using GralVm. Could you please review it again?

@dcore94
Copy link

dcore94 commented Oct 9, 2024

Sorry for jumping in but I'd like to ask that one think that I'm missing is the possibility that expression may be an array of strings where each array element represents a line of the expression. This would make writing the expression in JSON documents much much easier and more maintainable than using \n. Applies in general to INLINE Tasks...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants