Repository navigation
Say in the Element Data documentation what is safe with threads - #84
Merged
Merged
Conversation
This was referenced Nov 13, 2026
jwrosewell
marked this pull request as ready for review
November 14, 2026 20:49
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
For specifications#78, which has merged. thread-safety.md, Element Data, lines 56 and 57, says "Reading an Element Data instance once the Flow Element that writes it has finished MUST therefore be safe from several threads at once.", and lines 73 to 75 say "Code comments and documentation MUST be clear that these instances are not intended to be written from multiple threads simultaneously, nor read while they are being written." Nothing in this repository's documentation said anything about threads and Element Data.
What changed
The
ElementDataclass documentation inpipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/elementdata.pysays that only the Flow Element that creates an instance writes it, that a finished instance is safe to read from several threads at once, such as the caller's own threads, and that an instance written from several threads, or read while another thread writes it, is not catered for. The Python pipeline has no parallel groups, so the documentation does not mention them.Merge after #80
The documentation says a finished instance is safe to read from several threads at once. For the on-premise
SwigDataof Device Detection and IP Intelligence that is true only once #80 adds its lock, as lines 59 to 66 of the same page require, so this draft merges after #80, or with it.How it was verified
A test,
test_elementdata_documentation.py, checks that the documentation holds the statements.62cce3b3, run 42716837893, 1 passed, core 355 passed. The heads after it change only the clause about parallel groups, which the test does not read, and mergerc.tools/pr-matrix.pyontmp/pr-matrix-84, finished with success. Configure printed "Left out 5 macOS entries", all ten Build and Test jobs (Ubuntu and Windows, Python 3.10 to 3.14) passed, and all elevenSetting 'PullRequestSha'lines read045603b050cc9730e5329b674613304b0b2e47e7, this head. 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.Head 045603b0 is the earlier head 3d93705 with
rcat df0949d7 merged in, which merged cleanly.elementdata.pyis infiftyone_pipeline_core's source, so the release plan releases that package and every package that depends on it for this change, as it does for any change to the package's code.The last release into
mainwas #114, merged at 06:22 UTC on 13 November.