Repository navigation
Run the file system watcher on its own setting - #90
Merged
Merged
Conversation
The watcher test now expects a watcher with automatic updates on and off, and none with the watcher's own setting off. It replaces the test that no watcher is created while autoUpdate is off. The watched data files in dataFileUpdateService.test.js and the test that the watcher is logged were given autoUpdate, a polling interval and fake timers only to get a watcher, so they lose them again.
registerDataFile creates the watcher when fileSystemWatcher is set, whatever autoUpdate is, because watching a file downloads nothing. A data file with automatic updates off, such as one with no License Key, is watched again, as it is on main.
jwrosewell
marked this pull request as ready for review
November 13, 2026 00:15
This was referenced Nov 13, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On
rcthe file system watcher starts only when automatic updates are on. A deployment with automatic updates off that replaces its data file on disk gets no watcher, so the engine goes on with the data it loaded and the new file is not used. The published package watched the file in that case.An on-premise engine that uses the default update URL turns
autoUpdateoff when it has no License Key (deviceDetectionOnPremise.js lines 502 to 512, ipIntelligenceOnPremise.js lines 357 to 367). So onrcevery deployment with no License Key lost its watcher, although watching a file downloads nothing.mainat 384bff2,dataFileUpdateService.jsline 481, isif (dataFile.fileSystemWatcher) {. 384bff2 is the 4.5.102 tag, and line 481 ofdataFileUpdateService.jsin the publishedfiftyone.pipeline.engines4.5.102 on npm is the same.rcat 871e7a3,dataFileUpdateService.jsline 863, isif (dataFile.autoUpdate && dataFile.fileSystemWatcher) {. The condition came from #52, merged intorcon 10 November as aded0086, which also gave the watcher testsautoUpdateso that they would still get a watcher.The release draft #83 cannot ship until this is back the way
mainhas it.The rule
The file system watcher runs on its own setting and needs no License Key. The License Key is needed only for the two things that download, which are polling and the update on start-up. This rule was set on 12 November, and the specification now states it, as quoted below.
What the specification says
specifications#143 changed the data update page to state the rule, and merged into the specification's
rcon 12 November as 77dc1f62.data-updates.mdthere is identical to the draft's head d48add76, which this body quoted before the merge. The Automatic update master switch section, lines 99 to 109 saysand the Automatic update from File section, lines 182 to 186 says
Before #143 merged, the page on the specification's
rcat 48e28393 said the opposite. The master switch section, lines 99 to 105 said "Automatic update from File (file system watching) does not take place, even if a file system watcher is otherwise configured", and the Automatic update from File section, lines 178 to 181 said "With autoUpdate disabled, no file system watcher is created and changes to the data file on disk are not detected automatically."51degrees-go#110 made the same change in Go at e53aa8fc, with
TestTheWatcherRunsOnItsOwnSettingholding the watcher to its own setting in three cases, and three tests losing the automatic update setting they had been given only to get a watcher. This draft follows it.What changed
Two commits.
dataUpdateSpecification.test.jsthe test "is not created while autoUpdate is off" held the old rule. It is replaced by "runs on its own setting" with the three cases of go#110, being watcher on with automatic updates on, watcher on with automatic updates off, and watcher off with automatic updates on. Each case checks for the "Creating file system watcher" log entry and then whether a changed file is loaded within three seconds. The third case waits the full three seconds to show that nothing loads the file.autoUpdate, a polling interval and fake timers only to get a watcher, and loses them.dataFileUpdateService.test.jsthewatchedOptionsandregisterhelpers existed only to give the watched data filesautoUpdate, and to keep the polling timer thatautoUpdatestarts from running. They are removed. The three watcher tests and the watched data file of the collection test go back to{ fileSystemWatcher: true }andregisterDataFile, as they are onmain.registerDataFilecreates the watcher onfileSystemWatcheralone, and its comment says the watcher runs on its own setting whateverautoUpdateis, because watching a file downloads nothing.Nothing about polling, the update on start-up or the License Key changes. The engines still turn
autoUpdateandupdateOnStartoff with no License Key, and passfileSystemWatcherthrough unchanged, so they need no change.Red and green
Red. Full build run 43297550986 on 26e6b6f, the tests commit alone on
rc871e7a3.Ubuntu_ARM_Node_24failed with 5 failed, 2 skipped and 1339 passed, and the other five build jobs were cancelled when it failed. The five failures areExpected: trueandReceived: falseat the check for the "Creating file system watcher" entry, so no watcher was created,Received array: [],The other two cases of the new test passed.
Green. The full build below, on the head 98420f1, passes on all six build jobs with 1344 tests passed, 2 skipped and none failed out of 1346. No test fails, so the five that failed in the red run pass, and so do all three cases of the new test.
Full build on the head
Run 43300455731, on the throwaway branch tmp/pr-matrix-90b for head 98420f12, success. Read from the job logs:
Setting 'PullRequestSha'lines, in Configure and the six build jobs, are 98420f1.The draft's two checks on the head, Only rc into main and utm-link-lint, are green. The tmp/pr-matrix-90 and tmp/pr-matrix-90b branches are deleted. The branch is 0 commits behind
rc871e7a3.macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.
What remains
autoUpdateis on" and links #69. Once this draft is inrcthat sentence is no longer true. The condition came from Update the data file as the specification says #52, not Write the data update messages and levels the .NET service writes #69.automaticupdates/dataFileSystemWatcher.jsexamples still ask for a License Key and turnautoUpdateon, as they do onmain. This draft leaves them alone.