Add Jython Scripting #9404
Add Jython Scripting #9404
Conversation
Also-by: Scott Rushworth <openhab@5iver.com> Signed-off-by: Wouter Born <github@maindrain.net>
There was a problem hiding this comment.
Excellent, thanks!
The dependencies of Jython are indeed quite nasty and make the standalone jar pretty huge.
But I think we can live with the blown up KAR file size, although it's not ideal.
Although this is a feature PR, I'd definitely vote for merging it for RC2.
...cripting/src/main/java/org/openhab/automation/jythonscripting/JythonScriptEngineFactory.java
Outdated
Show resolved
Hide resolved
...cripting/src/main/java/org/openhab/automation/jythonscripting/JythonScriptEngineFactory.java
Outdated
Show resolved
Hide resolved
...cripting/src/main/java/org/openhab/automation/jythonscripting/JythonScriptEngineFactory.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Wouter Born <github@maindrain.net>
There was a problem hiding this comment.
Thanks for pushing this. I would love to see it in OH3 release.
I tested this port and it seems to be working smoothly. The only thing I do not understand is the recreation of the ScriptEngineManager. I asked it before but it could not be explained. But as it is working I am fine with it. See my linked comment inside.
| .get(OpenHAB.getConfigFolder(), "automation", "lib", "python").toString(); | ||
|
|
||
| private static final String SCRIPT_TYPE = "py"; | ||
| private static final javax.script.ScriptEngineManager ENGINE_MANAGER = new javax.script.ScriptEngineManager(); |
There was a problem hiding this comment.
Thanks for testing! The issue is that the org.openhab.core.automation.module.script.ScriptEngineFactory.ENGINE_MANAGER has a different classloader. So it only detects engines that are on the boot classpath (like Nashorn). I ran into the same issue with the Groovy add-on. I looked into ways to get engines registered in the ScriptEngineFactory.ENGINE_MANAGER. IIRC it wasn't possible to call methods on ScriptEngineFactory.ENGINE_MANAGER to (un)register engines provided by OSGi bundles.
There was a problem hiding this comment.
Do you have any concerns regarding
SecurityExceptionwhich might be thrown by this method?
We don't seem to handle any SecurityExceptions in openhab-core either when modifying System properties:
https://github.com/openhab/openhab-core/search?q=System.setProperty
There was a problem hiding this comment.
Do we need to handle it?
There was a problem hiding this comment.
Ah, okay. Sounds reasonable. Thanks for clarification.
Do we need to handle it?
I think it is okay to not handle it. If the exceptions will be thrown there might be a bigger issue regarding permissions and other stuff will fail too.
|
This seems to make the build unstable: |
* Add Jython Scripting Also-by: Scott Rushworth <openhab@5iver.com> Signed-off-by: Wouter Born <github@maindrain.net>
* Add Jython Scripting Also-by: Scott Rushworth <openhab@5iver.com> Signed-off-by: Wouter Born <github@maindrain.net>
* Add Jython Scripting Also-by: Scott Rushworth <openhab@5iver.com> Signed-off-by: Wouter Born <github@maindrain.net> Signed-off-by: Joseph Hagberg <joseph@zoidberg.se>
* Add Jython Scripting Also-by: Scott Rushworth <openhab@5iver.com>
* Add Jython Scripting Also-by: Scott Rushworth <openhab@5iver.com> Signed-off-by: Wouter Born <github@maindrain.net>
Ports #7208 to OH3 and addresses review comments like removing the bnd.bnd file and using an annotation for the dynamic import.
I'm not a Jython expert but some Hello World examples seemed to work in rules so at least something is functional.
Perhaps some seasoned Jython users can further test and improve it?
As expected it is also a pretty big add-on with a size of 42 MB, so it will have a big impact on the add-ons KAR file.
See also #4801 (comment)