[automation] ScriptFileWatcher Initial import now only processes dir #2708
Conversation
Signed-off-by: Jonathan Gilbert <jpg@trillica.com>
|
@digitaldan would you be able to test this? I seemed to be working for me, but I've had a few issues with deployment and wanted a second check if possible. Thanks (oh - it fixes the initial import upon startup problem). |
cweitkamp
added
automation
bug
|
Just gave it a try, and yes this does indeed fix the issue, thanks! |
| @@ -285,7 +303,7 @@ private synchronized void onStartLevelChanged(int newLevel) { | |||
| if (newLevel >= StartLevelService.STARTLEVEL_MODEL) { // start | |||
| ScheduledExecutorService localScheduler = executorFactory.get(); | |||
| scheduler = localScheduler; | |||
| localScheduler.submit(() -> importResources(new File(pathToWatch))); | |||
| localScheduler.submit(() -> importInitialResources(new File(pathToWatch))); | |||
| localScheduler.scheduleWithFixedDelay(() -> checkFiles(currentStartLevel), 0, RECHECK_INTERVAL, | |||
There was a problem hiding this comment.
Could it be that the missing initial delay causes the issues mentioned in #2727? If you do not import everything in the line above, and start this without any delay it could be that something is missing when checkFiles is called for the first time and therefore something is removed from pending too early or not removed from pending and therefore ignored in later checks?
Otherwise I don't see how this change could result in something not being loaded, except it runs into something like #2466, which worked before because it was loaded on startup?
There was a problem hiding this comment.
Regarding this change specifically, the only effective logic change is that at startup directories are not recursively scanned iff watchSubDirectories() returns false. If it is set to true, then the behaviour should be identical.
As for whether there is a race happening somewhere between both of these import processes (importInitialResources and checkFiles), this could indeed be the case, but should not have changed as a result of this PR. However, now that there is an explicit importInitialResources, it would be possible to synchronize (on pending) as part of this method, plus even (optionally) move the scheduling of checkFiles into that method. By adding the synchronisation in importInitialResources, then checkFiles will need to wait for it to complete entirely before it can run.
(tbh I believe that this class needs a rewrite, and would personally make it abstract and push the responsibility of creating the osgi components to language implementations rather than having a combined all-language watcher. I pulled JS out of this into the JS plugin, but didn't want to take on doing if for the other languages.)
…penhab#2708) Signed-off-by: Jonathan Gilbert <jpg@trillica.com> GitOrigin-RevId: 290ad08
ScriptFileWatcher reused logic for recursion and initial import. Initial import always processes a directory and it's contents (only), whereas ongoing monitoring may choose not to descend into subdirs, and processes files too. This change separates the logic.
Signed-off-by: Jonathan Gilbert jpg@trillica.com