Skip to content

Say in the Element Data documentation what is safe with threads - #84

Merged
jwrosewell merged 5 commits into
rcfrom
fix/element-data-thread-docs
Nov 14, 2026
Merged

jwrosewell merged 5 commits into
rcfrom
fix/element-data-thread-docs

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

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 ElementData class documentation in pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/elementdata.py says 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 SwigData of 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.

  • Red, run 42716723558, 1 failed.
  • Green, head 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 merge rc.
  • Full build, run 43478536718, started by tools/pr-matrix.py on tmp/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 eleven Setting 'PullRequestSha' lines read 045603b050cc9730e5329b674613304b0b2e47e7, 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 rc at df0949d7 merged in, which merged cleanly.

elementdata.py is in fiftyone_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 main was #114, merged at 06:22 UTC on 13 November.

@jwrosewell
jwrosewell marked this pull request as ready for review November 14, 2026 20:49
@jwrosewell
jwrosewell merged commit 5278e81 into rc Nov 14, 2026
3 checks passed
@jwrosewell
jwrosewell deleted the fix/element-data-thread-docs branch November 14, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant