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

Fix caching in WorkflowLocal/WorkflowThreadLocal (#1876) #1878

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

dano
Copy link
Contributor

@dano dano commented Oct 2, 2023

What was changed

Reverted caching changes made to WorkflowLocal/WorkflowThreadLocal, which broke backwards compatibility and accidentally shared values between Workflows/Threads. Re-implemented caching as an optional feature, and deprecated the factory methods that created non-caching instances.

Why?

The current implementation both broke backwards compatibility, and did not function correctly; it shared values between Workflows and Threads when it should not have. See #1876 for details.

Checklist

  1. Closes WorkflowLocal shares initial value between all Workflows #1876

  2. How was this tested:
    Unit tests

  3. Any docs updates needed?
    I don't think WorkflowLocal/WorkflowThreadLocal are documented outside of Javadocs, so no.

@dano dano requested a review from a team as a code owner October 2, 2023 17:30
@CLAassistant
Copy link

CLAassistant commented Oct 2, 2023

CLA assistant check
All committers have signed the CLA.

Reverted caching changes made to WorkflowLocal/WorkflowThreadLocal,
which broke backwards compatibility and accidentally shared values
between Workflows/Threads. Re-implemented caching as an optional
feature, and deprecated the factory methods that created non-caching
instances.
@dano dano force-pushed the fix-wflocal-caching branch from 9c0cc84 to a83eb28 Compare October 2, 2023 21:47
Copy link
Contributor

@Quinn-With-Two-Ns Quinn-With-Two-Ns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @dano

@Quinn-With-Two-Ns Quinn-With-Two-Ns merged commit 198c780 into temporalio:master Oct 2, 2023
4 checks passed
@dano dano deleted the fix-wflocal-caching branch October 3, 2023 13:44
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.

WorkflowLocal shares initial value between all Workflows
3 participants