diff --git a/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py b/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py index 7d5c6972..6366e769 100644 --- a/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py +++ b/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py @@ -542,12 +542,10 @@ def register_data_file(self, data_file): if data_file.update_on_start or data_file.auto_update: self.check_next_update(data_file) - # Watch for changes in file and refresh if - # file_system_watcher is enabled on data_file - - # The watcher is an automatic update, so it only runs with - # automatic updates on. - if data_file.auto_update and data_file.file_system_watcher: + # The file system watcher runs on its own setting, whether or not + # automatic updates are on. It downloads nothing, so it needs no + # licence key. + if data_file.file_system_watcher: self._log("info", "Creating file system watcher") data_file.last_changed_time = os.stat(data_file.path).st_mtime self.track_local_data_file_changes(data_file) diff --git a/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_http.py b/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_http.py index fa30d679..9d3dba02 100644 --- a/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_http.py +++ b/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_http.py @@ -450,27 +450,11 @@ def testFileSystemWatcherIsLogged(self): with open(data_file.path, "w") as file: file.write("x") data_file.file_system_watcher = True - data_file.auto_update = True self.hold_timers() self.service.register_data_file(data_file) self.assertIn( ("info", "Creating file system watcher"), self.pipeline.logs) - def testNoFileSystemWatcherWithoutAutomaticUpdates(self): - folder = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, folder, ignore_errors=True) - data_file = HttpDataFile(os.path.join(folder, "data.dat")) - with open(data_file.path, "w") as file: - file.write("x") - data_file.file_system_watcher = True - data_file.auto_update = False - self.hold_timers() - self.service.register_data_file(data_file) - self.assertNotIn( - ("info", "Creating file system watcher"), self.pipeline.logs) - self.assertEqual([], self.pending) - self.assertIsNone(getattr(data_file, "watcher_timer", None)) - # A data file kept on disk that cannot put the new file in place, and one # with no temporary file path, carry .NET's message and status. diff --git a/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_service.py b/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_service.py index 96dd178f..b96f9b5f 100644 --- a/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_service.py +++ b/pipeline/fiftyone_pipeline_engines/tests/test_datafile_update_service.py @@ -119,19 +119,48 @@ def run_next_check(self): check = self.pending.pop(0) check.function(*check.args) - def register_watched(self, data_file): - # The watcher runs only with automatic updates on. The update checks - # those would schedule are left out, so only the watcher's are - # pending. - data_file.auto_update = True - with mock.patch.object(self.service, "check_next_update"): - self.service.register_data_file(data_file) + # The watcher runs on its own setting, whether or not automatic updates + # are on. Watching a file downloads nothing, so a data file with no + # licence key, whose automatic updates are off, is still watched. + def testTheWatcherRunsOnItsOwnSetting(self): + for name, automatic, watcher in ( + ("watcher on, automatic updates on", True, True), + ("watcher on, automatic updates off", False, True), + ("watcher off, automatic updates on", True, False)): + with self.subTest(name): + self.write("version 1") + self.pending.clear() + pipeline = RecordingPipeline() + service = DataFileUpdateService(pipeline) + data_file = StandInDataFile(self.path) + data_file.auto_update = automatic + data_file.file_system_watcher = watcher + data_file.polling_interval = 30 + data_file.update_time_maximum_randomisation = 0 + service.register_data_file(data_file) + + watches = [ + check for check in self.pending + if check.function == service.track_local_data_file_changes] + self.assertEqual(1 if watcher else 0, len(watches), name) + self.assertEqual( + watcher, + ("info", "Creating file system watcher") in pipeline.logs, + name) + + self.write("version 2") + for check in watches: + check.function(*check.args) + self.assertEqual( + "version 2" if watcher else "version 1", data_file.data, + name) + service.unregister_data_file(data_file) # A refresh that fails is logged and reported, the data in use is kept, # and a later good file is picked up. def testFileSystemWatcherKeepsWatchingAfterFailedRefresh(self): data_file = StandInDataFile(self.path) - self.register_watched(data_file) + self.service.register_data_file(data_file) self.write("unusable") self.run_next_check() @@ -156,7 +185,7 @@ def testFileSystemWatcherKeepsWatchingAfterFailedRefresh(self): # reports success to the update complete callbacks. def testFileSystemWatcherRefreshIsRoutedAsPollingIs(self): data_file = StandInDataFile(self.path) - self.register_watched(data_file) + self.service.register_data_file(data_file) self.write("version 2") self.run_next_check() @@ -172,7 +201,7 @@ def testFileSystemWatcherRefreshIsRoutedAsPollingIs(self): # the specification's errors table, as the polling route's does. def testFileSystemWatcherFailedRefreshUsesTheTableMessage(self): data_file = StandInDataFile(self.path) - self.register_watched(data_file) + self.service.register_data_file(data_file) self.write("unusable") self.run_next_check() @@ -189,7 +218,7 @@ def testFileSystemWatcherFailedRefreshUsesTheTableMessage(self): # replaced, is looked for again at the next check. def testFileSystemWatcherKeepsWatchingWhileFileIsMissing(self): data_file = StandInDataFile(self.path) - self.register_watched(data_file) + self.service.register_data_file(data_file) os.remove(self.path) self.run_next_check() @@ -206,7 +235,7 @@ def testFileSystemWatcherKeepsWatchingWhileFileIsMissing(self): # nothing is left holding the data file. def testUnregisterStopsFileSystemWatcher(self): data_file = StandInDataFile(self.path) - self.register_watched(data_file) + self.service.register_data_file(data_file) check = self.pending[0] self.service.unregister_data_file(data_file)