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

Add WorkerStateManager for RedisStateManager #1022

Closed
wants to merge 3 commits into from

Conversation

zbirenbaum
Copy link
Contributor

@zbirenbaum zbirenbaum commented Jun 18, 2024

Description

Implements WorkerStateManager trait for RedisStateManager. Adds API hooks for worker-driven updates to operation state.

Requires: #1020 #1021

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@zbirenbaum zbirenbaum mentioned this pull request Jun 18, 2024
5 tasks
@zbirenbaum zbirenbaum force-pushed the redis-worker-state branch 3 times, most recently from 67864cb to f46b3d4 Compare June 18, 2024 21:21
Copy link
Contributor

@adam-singer adam-singer left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04)

@zbirenbaum zbirenbaum force-pushed the redis-worker-state branch from f46b3d4 to 88b9aaa Compare June 19, 2024 17:52
@zbirenbaum zbirenbaum mentioned this pull request Jun 19, 2024
4 tasks
Copy link
Contributor

@adam-singer adam-singer left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 7 files at r1, 3 of 3 files at r4, 1 of 1 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 LGTMs obtained

@zbirenbaum zbirenbaum requested a review from allada June 19, 2024 20:53
@zbirenbaum zbirenbaum force-pushed the redis-worker-state branch 5 times, most recently from 74ed7d6 to 9b95726 Compare June 25, 2024 06:09
Creates groundwork for Redis State Manager implementation by introducing
two new structs. RedisOperation represents an operation stored in redis
and RedisOperationState represents the wrapped object which implements
the ActionStateResult trait.
Adds RedisStore based implementation for ClientStateManager trait.
Provides API hooks for adding or filtering Operations.
Implements WorkerStateManager trait for RedisStateManager. Adds API
hooks for worker-driven updates to OperationState.
@zbirenbaum zbirenbaum force-pushed the redis-worker-state branch from 9b95726 to 499fc28 Compare June 25, 2024 06:26
@zbirenbaum
Copy link
Contributor Author

Changes merged in #1023

@zbirenbaum zbirenbaum closed this Jun 28, 2024
@allada allada deleted the redis-worker-state branch July 28, 2024 19:11
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.

2 participants