Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust plugin thread-local APIs to account for WorkunitStore state #15887

Closed
stuhood opened this issue Jun 21, 2022 · 0 comments · Fixed by #15890
Closed

Adjust plugin thread-local APIs to account for WorkunitStore state #15887

stuhood opened this issue Jun 21, 2022 · 0 comments · Fixed by #15890
Milestone

Comments

@stuhood
Copy link
Member

stuhood commented Jun 21, 2022

When eager_fetch=False, it's possible that a workunit's "artifacts" contain Digests which haven't actually been fetched. When that's the case for a Digest, and a StreamingWorkunit plugin is using any of the context methods which fetch files from a background thread, they will encounter a:

A WorkunitStore has not been set for this thread.

...error. That's because our existing native_engine.stdio_thread_set_destination statics only set the thread local stdio state, and not also our workunit state.


To fix this, we should adjust the existing method to additionally set the workunit store. But we should also deprecate the existing method and add a new one with a more accurate name (replacing #12295).

@stuhood stuhood added this to the 2.13.x milestone Jun 21, 2022
stuhood added a commit that referenced this issue Jun 23, 2022
…te stdio-specific methods (#15890)

As described in #15887: `StreamingWorkunit` plugins have never been able to set thread-local `WorkunitStore` state, but that apparently didn't matter until #11331 made it possible for the `StreamingWorkunitContext` file-fetching methods to encounter data which had not yet been fetched (and thus needed to create a workunit for the fetching).

This change updates and "deprecates" the existing `stdio_thread_[gs]et_destination` methods (although it doesn't have access to a decorator to do that), and introduces generic thread-local state methods which include all thread-local state required by engine APIs.

Fixes #15887.

[ci skip-build-wheels]
stuhood added a commit to stuhood/pants that referenced this issue Jun 23, 2022
…te stdio-specific methods (pantsbuild#15890)

As described in pantsbuild#15887: `StreamingWorkunit` plugins have never been able to set thread-local `WorkunitStore` state, but that apparently didn't matter until pantsbuild#11331 made it possible for the `StreamingWorkunitContext` file-fetching methods to encounter data which had not yet been fetched (and thus needed to create a workunit for the fetching).

This change updates and "deprecates" the existing `stdio_thread_[gs]et_destination` methods (although it doesn't have access to a decorator to do that), and introduces generic thread-local state methods which include all thread-local state required by engine APIs.

Fixes pantsbuild#15887.

[ci skip-build-wheels]
stuhood added a commit that referenced this issue Jun 23, 2022
…te stdio-specific methods (Cherry-pick of #15890) (#15907)

As described in #15887: `StreamingWorkunit` plugins have never been able to set thread-local `WorkunitStore` state, but that apparently didn't matter until #11331 made it possible for the `StreamingWorkunitContext` file-fetching methods to encounter data which had not yet been fetched (and thus needed to create a workunit for the fetching).

This change updates and "deprecates" the existing `stdio_thread_[gs]et_destination` methods (although it doesn't have access to a decorator to do that), and introduces generic thread-local state methods which include all thread-local state required by engine APIs.

Fixes #15887.

[ci skip-build-wheels]
stuhood added a commit to stuhood/pants that referenced this issue Jun 24, 2022
…te stdio-specific methods (pantsbuild#15890)

As described in pantsbuild#15887: `StreamingWorkunit` plugins have never been able to set thread-local `WorkunitStore` state, but that apparently didn't matter until pantsbuild#11331 made it possible for the `StreamingWorkunitContext` file-fetching methods to encounter data which had not yet been fetched (and thus needed to create a workunit for the fetching).

This change updates and "deprecates" the existing `stdio_thread_[gs]et_destination` methods (although it doesn't have access to a decorator to do that), and introduces generic thread-local state methods which include all thread-local state required by engine APIs.

Fixes pantsbuild#15887.

[ci skip-build-wheels]
stuhood added a commit that referenced this issue Jun 24, 2022
…te stdio-specific methods (Cherry-pick of #15890) (#15916)

As described in #15887: `StreamingWorkunit` plugins have never been able to set thread-local `WorkunitStore` state, but that apparently didn't matter until #11331 made it possible for the `StreamingWorkunitContext` file-fetching methods to encounter data which had not yet been fetched (and thus needed to create a workunit for the fetching).

This change updates and "deprecates" the existing `stdio_thread_[gs]et_destination` methods (although it doesn't have access to a decorator to do that), and introduces generic thread-local state methods which include all thread-local state required by engine APIs.

Fixes #15887.

[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant