[automation] Script file watchers now reusable for language-specific versions #2593
Conversation
|
Related PR to actually use this: openhab/openhab-addons#11719 |
...nhab/core/automation/module/script/rulesupport/internal/loader/DefaultScriptFileWatcher.java
Outdated
Show resolved
Hide resolved
jpg0
force-pushed
the
reusable-file-watchers
branch
from
1612e6c to
d8089fc
Compare
|
@cweitkamp and @kaikreuzer we have a number of changes we are trying to get in to provide proper support for the GraalVM script engine. This PR as well as #2595 will allow us to use both Nashorn and GraalVM, side by side, in both UI rules as well as files. Right now the two engines kinda clobber each other and are hurting us more then helping when you try and install GraalVM. This should really help us start to transition to GraalVM as our preferred scripting engine and eventually remove Nashorn in a graceful way over time. |
jpg0
force-pushed
the
reusable-file-watchers
branch
from
d8089fc to
0b17a7f
Compare
jpg0
changed the title
|
Any chance of a review or feedback @kaikreuzer @cweitkamp ? Thanks! |
There was a problem hiding this comment.
Thanks. I support the idea behind this PR. I like to see a review from @kaikreuzer as this has been discussed with him earlier.
I left two questions for my personal understanding.
...ain/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcher.java
Show resolved
Hide resolved
...ain/java/org/openhab/core/automation/module/script/rulesupport/loader/DependencyTracker.java
Show resolved
Hide resolved
|
@kaikreuzer bump! |
There was a problem hiding this comment.
Thanks, code looks good to me, but I haven't tested it - I assume you did.
Just some minor style comments below and also waiting for @cweitkamp's approval of it.
...nhab/core/automation/module/script/rulesupport/internal/loader/DefaultScriptFileWatcher.java
Outdated
Show resolved
Hide resolved
|
Thanks! Yes i have been testing it for about a week without issues. |
...ain/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcher.java
Outdated
Show resolved
Hide resolved
…ic versions Signed-off-by: Jonathan Gilbert <jpg@trillica.com>
jpg0
force-pushed
the
reusable-file-watchers
branch
from
0b17a7f to
dddc8b0
Compare
| @@ -111,6 +103,21 @@ public void deactivate() { | |||
| super.deactivate(); | |||
| } | |||
|
|
|||
| @Override | |||
| public void activate() { | |||
|
|
|||
|
So this builds and tests fine on my machine, is the failing build is due to flakiness in the integration tests? If so should I force re-push with no material changes to force another build? Or can we ignore it? |
|
As the PR build on Jenkins succeeded, I guess we can ignore it. |
…ic versions (openhab#2593) Signed-off-by: Jonathan Gilbert <jpg@trillica.com> GitOrigin-RevId: 2883dfb
Extracted reusable parts of script watching to allow language-specific instantiations:
This change is specifically to prepare for JS-specific scripts to operate as standard NodeJS which is slightly different to the existing openHAB mechanism, as discussed with @kaikreuzer. No functionality change to core with this change, except the creation of the script directory if not already present. PR to use this functionality in openhab-addons to follow.
Signed-off-by: Jonathan Gilbert jpg@trillica.com