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

Script files not executed in order at startup #2002

Closed
jpg0 opened this issue Dec 24, 2020 · 8 comments · Fixed by #2222
Closed

Script files not executed in order at startup #2002

jpg0 opened this issue Dec 24, 2020 · 8 comments · Fixed by #2222

Comments

@jpg0
Copy link
Contributor

jpg0 commented Dec 24, 2020

In OH2.5 script files were executed in order (alphanumeric based on script name) at startup. In OH3 there appears to be no specific order in their initial execution.

Looking at the code, I believe that the problem is in https://github.com/openhab/openhab-core/blob/master/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptFileWatcher.java

In 2.5, the watcher starts early, before script engines are available, and with a specific "earliestStart" time, and enqueues all the script files. When the engine becomes available, the files are dequeued, sorted (in checkFiles), then passed on for execution. Note that this is a pretty opaque mechanism to sort the files - the method doesn't mention it, and it only happens dues to getting enqueued before the script engine is available/earliestStart is reached (if the script engine is available, they get submitted immediately in the order they are read/detected). It looks like in OH3 this behaviour was missed when adding the ReadyState stuff, because the first thing called is importFiles rather than checkFiles, which, if script engines are available, will not submit the scripts in alphanumeric order.

@kaikreuzer
Copy link
Member

That's indeed a very opaque feature that wasn't clear to me when working on the start levels. It also does not sound like a feature I like much as expecting certain processing orders wrt filenames is something pretty unusual for any file handling system...

@jpg0
Copy link
Contributor Author

jpg0 commented Dec 25, 2020

I knew about it because it's mentioned in the docs. Also my scripts do rely on it: there is one script which executes first that sets up all the items that others operate on. So my use-case is really dependencies rather the strict execution order, although expressing order is certainly simple in terms of configuration.

I noticed that this broke in OH3 and even knowing the intended behaviour, it took me a while to track down where it was even supposed to be happening in the code. If we intend to preserve the existing behaviour, I'd suggest the ordering is made explicit in the code. (If I find time I'll submit a PR.)

@kaikreuzer
Copy link
Member

Imho, dependencies should be resolved in other ways - e.g. by using the new start levels.

@jpg0
Copy link
Contributor Author

jpg0 commented Dec 26, 2020

I think that could work, just as long as we keep it simple (by default) for scripts.

For things to execute after the majority of other scripts, it would be possible for a script to register at a later level in the script itself. For scripts that want to run before other scripts it's less simple (as every other script would need to be modified).

One option would be to do something a little like Unix runlevels and init.d - if you put your script in a folder denoting the desired start level then it's run then instead of the default.

There do seem to be a few bugs associated with scripts starting too early, so maybe assigning scripts to explicit start levels (with a late default) would fix these too.

@LukasA83
Copy link

I also found rules / scripts in files are getting loaded over the complete startlevel range, starting with 20 or 30 up to / beyond 100, which makes the startlevel trigger unreliable for file provisioned rules. I guess this is not the intended behaviour. I'd assume on startlevel 40 or 50 all file provisioned rules are loaded, too.

@jpg0
Copy link
Contributor Author

jpg0 commented Jan 2, 2021

@LukasA83 I agree - I would think that the default should be 40 or 50, and allow individual scripts to override this for themselves (whilst keeping the script watcher itself at 20).

@jpg0
Copy link
Contributor Author

jpg0 commented Jan 29, 2021

@kaikreuzer I'm planning to have a go at fixing this as it's blocking me upgrading to 3.x. The simplest fix would be to just sort the filenames alphanumerically like happened in 2.x. This would also match the previous behaviour. However if you don't want this and would prefer a solution using start-levels I can certainly have a go at that instead (although would want to discuss how scripts assign themselves to levels). Another option would be to do both - first sort by filename, then add assignment to start level too (where within one start level they would get sorted by filename).

@jpg0
Copy link
Contributor Author

jpg0 commented Mar 3, 2021

@kaikreuzer I've gone ahead and implemented your suggestion - to allow scripts to specify their own start levels if they need to do so. PR is ready for review #2222

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

Successfully merging a pull request may close this issue.

4 participants