[Observables] chore: add ReplayObservable#SubscribeFromLatestBufferedOffset()
#647
Conversation
bryanchriswhite
added
off-chain
WalkthroughThe changes introduce a method called Changes
Sequence Diagram(s)sequenceDiagram
participant Observer
participant ReplayObservable
participant Buffer
Observer->>ReplayObservable: SubscribeFromLatestBufferedOffset(offset)
ReplayObservable->>Buffer: Check Offset
Buffer-->>ReplayObservable: Valid/Invalid Offset
alt Valid Offset
ReplayObservable->>Observer: Emit Buffered Values from Offset
ReplayObservable->>Observer: Subscribe to New Values
else Invalid Offset
ReplayObservable->>Observer: Error or Emit All Buffered Values
ReplayObservable->>Observer: Subscribe to New Values
end
Possibly related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. You may need to run GCP workloads (requires changing the namespace to 647) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- pkg/observable/channel/replay.go (2 hunks)
- pkg/observable/channel/replay_test.go (2 hunks)
- pkg/observable/interface.go (1 hunks)
Additional comments not posted (5)
pkg/observable/interface.go (1)
14-22: LGTM!The addition of the
SubscribeFromLatestBufferedOffsetmethod to theReplayObservableinterface is consistent and well-documented.pkg/observable/channel/replay.go (2)
125-127: LGTM!The update to the
Subscribemethod to useSubscribeFromLatestBufferedOffsetis straightforward and aligns with the new functionality.
Line range hint
129-169: LGTM!The
SubscribeFromLatestBufferedOffsetmethod is well-implemented. The logic ensures that the offset is within bounds and replays the values accordingly. The use of goroutines and context management is appropriate.pkg/observable/channel/replay_test.go (2)
305-360: LGTM!The
TestReplayObservable_SubscribeFromLatestBufferedOffsettest function is comprehensive and covers various scenarios. The use of theaccumulateValueshelper function is appropriate.
362-379: LGTM!The
accumulateValueshelper function is well-implemented and aids in accumulating values from the observer.
bryanchriswhite
changed the title
ReplayObservable#SubscribeFromLatestBufferedOffset()
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- pkg/observable/channel/replay.go (1 hunks)
- pkg/observable/channel/replay_test.go (2 hunks)
- pkg/observable/interface.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- pkg/observable/channel/replay.go
- pkg/observable/channel/replay_test.go
- pkg/observable/interface.go
bryanchriswhite
removed
push-image
| name: "endOffset > replayBufferSize", | ||
| replayBufferSize: 8, | ||
| endOffset: 10, | ||
| expectedValues: values[2:], |
There was a problem hiding this comment.
I don't understand why this doesn't start at values[len(values)-1:]?
There was a problem hiding this comment.
It's values[len(values)-replayBufferSize].
// Published values
[ 1 2 3 4 5 ]
// Replay buffer
[ 5 4 3 2 1 ]
// SubscribeFromEndOffset(100)
[ 1 2 3 4 5 ]
// SubscribeFromLatestBufferedOffset(5)
[ 1 2 3 4 5 ]
// SubscribeFromLatestBufferedOffset(3)
[ 3 4 5 ]
| for i := len(ro.replayBuffer) - 1; i >= 0; i-- { | ||
|
|
||
| // Ensure that the offset is within the bounds of the replay buffer. | ||
| if endOffset > len(ro.replayBuffer) { |
There was a problem hiding this comment.
This makes it a bit confusing if endOffset is number of elements from the start or the end of the buffer.
Should we PUC that "it points to the last element in the replayBuffer"?
There was a problem hiding this comment.
It's an offset from the "latest buffered" value. I thought the name and godoc comment would make it sufficiently clear. I'm not sure I understand your suggestion; what is pointing to the last element in the replayBuffer? And by last do you mean "latest" or largest index?
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- pkg/observable/channel/replay.go (1 hunks)
- pkg/observable/channel/replay_test.go (2 hunks)
- pkg/observable/interface.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/observable/channel/replay_test.go
Additional context used
golangci-lint
pkg/observable/interface.go
19-19: File is not
goimports-ed(goimports)
pkg/observable/channel/replay.go
133-133: File is not
goimports-ed(goimports)
Additional comments not posted (5)
pkg/observable/interface.go (1)
14-24: LGTM! Clear and detailed documentation.The documentation for
SubscribeFromLatestBufferedOffsetis clear and provides detailed information about the method's behavior.Tools
golangci-lint
19-19: File is not
goimports-ed(goimports)
pkg/observable/channel/replay.go (4)
125-126: LGTM! The changes to theSubscribemethod are correct.The
Subscribemethod now callsSubscribeFromLatestBufferedOffsetwithro.replayBufferSize, which is appropriate.
128-138: LGTM! Clear and detailed documentation.The documentation for
SubscribeFromLatestBufferedOffsetis clear and provides detailed information about the method's behavior.Tools
golangci-lint
133-133: File is not
goimports-ed(goimports)
138-153: LGTM! The implementation ofSubscribeFromLatestBufferedOffsetis correct.The method implementation correctly handles subscribing from the latest buffered offset and ensures the offset is within bounds.
153-154: LGTM! The loop logic is correct.The loop correctly replays values from the buffer in reverse order.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- pkg/observable/channel/replay.go (1 hunks)
- pkg/observable/interface.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- pkg/observable/channel/replay.go
- pkg/observable/interface.go
bryanchriswhite
added
push-image
|
The image is going to be pushed after the next commit. You can use If you also want to run E2E tests, please add |
|
The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. You may need to run GCP workloads (requires changing the namespace to 647) |
…session-manager * pokt/main: [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668)
…ent-balances * pokt/main: [TODOs] refactor: proof path calculation (#659) [Dependencies] bump go-getter and ibc-go (#691) [Relayminer] refactor: `relayerSessionsManager#waitForBlock()` (#648) [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668) build(deps): bump ws from 7.5.9 to 7.5.10 in /docusaurus (#686) build(deps): bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /docusaurus (#688) build(deps): bump express from 4.18.2 to 4.19.2 in /docusaurus (#687) build(deps): bump follow-redirects from 1.15.3 to 1.15.6 in /docusaurus (#685) build(deps): bump braces from 3.0.2 to 3.0.3 in /docusaurus (#689) [CosmosSDK] Bump to v0.50.7 (#682)
…ation-overserviced * pokt/main: [TODOs] refactor: proof path calculation (#659) [Dependencies] bump go-getter and ibc-go (#691) [Relayminer] refactor: `relayerSessionsManager#waitForBlock()` (#648) [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668) build(deps): bump ws from 7.5.9 to 7.5.10 in /docusaurus (#686) build(deps): bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /docusaurus (#688) build(deps): bump express from 4.18.2 to 4.19.2 in /docusaurus (#687) build(deps): bump follow-redirects from 1.15.3 to 1.15.6 in /docusaurus (#685) build(deps): bump braces from 3.0.2 to 3.0.3 in /docusaurus (#689) [CosmosSDK] Bump to v0.50.7 (#682)
…ation-use-index * pokt/main: [TODOs] refactor: proof path calculation (#659) [Dependencies] bump go-getter and ibc-go (#691) [Relayminer] refactor: `relayerSessionsManager#waitForBlock()` (#648) [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668) build(deps): bump ws from 7.5.9 to 7.5.10 in /docusaurus (#686) build(deps): bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /docusaurus (#688) build(deps): bump express from 4.18.2 to 4.19.2 in /docusaurus (#687) build(deps): bump follow-redirects from 1.15.3 to 1.15.6 in /docusaurus (#685) build(deps): bump braces from 3.0.2 to 3.0.3 in /docusaurus (#689) [CosmosSDK] Bump to v0.50.7 (#682)
…dOffset()` (#647) Co-authored-by: Daniel Olshansky <[email protected]>
Summary
Add
ReplayObservable#SubscribeFromLatestBufferedOffset()to support finer control over replay observable subscriptions.Issue
ReplayObservable#SubscribeFromBufferEndOffset()method #553Type of change
Select one or more:
Testing
Documentation changes (only if making doc changes)
make docusaurus_start; only needed if you make doc changesLocal Testing (only if making code changes)
make go_develop_and_testmake test_e2ePR Testing (only if making code changes)
devnet-test-e2elabel to the PR.make trigger_ciif you want to re-trigger tests without any code changesSanity Checklist
Summary by CodeRabbit