[Observable] chore: observable touchup #83

bryanchriswhite · 2023-10-20T09:46:37Z

Summary

Human Summary

add Observer#IsClosed() to prevent redundant unsubscription
simplify channelObservable
update observable pkg README
update pkg README template
add Observable#Next() conveniece method to just get the next value

AI Summary

Summary generated by Reviewpad on 23 Oct 23 16:25 UTC

This pull request includes the following changes:

In the file docs/pkg/observable/README.md, the package path has been updated from pkg/observable to pocket/pkg/observable.
In the file docs/template/pkg/README.md, the section "Architecture Diagrams" has been added to provide visual representations of the package's design and flow.
In the file pkg/observable/channel/observable.go, the Next method has been added to synchronously return the next value from the observable.
In the file pkg/observable/channel/observable.go, the goPublish function has been modified to use the publishCh field of the channelObservable struct.
In the file pkg/observable/channel/observable_test.go, new test cases have been added to test different scenarios of the NotifyObservers function.
In the file pkg/observable/channel/observable_test.go, the UnsubscribeObservers function now logs a warning when the observer is closed.
In the file pkg/observable/channel/observer.go, the IsClosed method has been added to check if the observer has been unsubscribed.
In the file pkg/observable/interface.go, the Next method has been added to the Observable interface to synchronously return the next value from the observable.
In the file pkg/observable/interface.go, the IsClosed method has been added to the Observer interface to check if the observer has been unsubscribed.

Issue

Follow-up to [Pkg] feat: add channel observable #31

[Explain the reasoning for the PR in 1-2 sentences. Consider adding a link or a screenshot.]

Type of change

Select one or more:

Testing

Run all unit tests: make go_test
Verify Localnet manually: See the instructions [here](TODO: add link to instructions)

Sanity Checklist

I have tested my changes using the available tooling
I have performed a self-review of my own code
I have commented my code, updated documentation and left TODOs throughout the codebase

(cherry picked from commit 78a9946b3f14353e79b123919416903d4622da4d)

(cherry picked from commit a2629c8bc3decfb5a787e453af67aa78fc8ca1ea)

(cherry picked from commit e97b691e39af8fa1654b8d697a3b34095b32ed82)

(cherry picked from commit d5442c7062630d847e048850fa71806086f84172)

bryanchriswhite · 2023-10-23T16:00:31Z

Will need to merge main once #91 is in to pass CI.

(cherry picked from commit cb4142f673fee37ead8520394e314f1fcb9d0dc9)

* pokt/main: fix: RelayerProxy interface mismatch (#91)

Olshansk · 2023-10-23T16:17:01Z

docs/template/pkg/README.md

@@ -1,6 +1,3 @@
-Certainly! I've added a section named "Architecture Diagrams" in the documentation template below:
-
-```markdown
 # Package [PackageName]

 > Brief one-liner or quote about what this package does.


Do you mean to add TODOs here?

Olshansk · 2023-10-23T16:17:15Z

docs/template/pkg/README.md

@@ -16,11 +13,21 @@ Provide a few sentences about the purpose and functionality of this package. Con

 Visual representations often make it easier to understand the design and flow of a package. Below are the architecture diagrams that explain the high-level structure and interactions in this package:

-![Architecture Overview](./path-to-diagram1.png)
+```mermaid


Are these changes intended?

Yes, this file is intended to be a template for generating go package READMEs with LLMs.

Olshansk · 2023-10-23T16:17:28Z

docs/template/pkg/README.md

+title: Architecture Overview
+---
+flowchart
+```

 > **Figure 1**: Brief description about what this diagram represents.


Plaeholder?

Olshansk · 2023-10-23T16:17:47Z

pkg/observable/channel/observable.go

@@ -63,6 +64,14 @@ func WithPublisher[V any](publishCh chan V) option[V] {
 	}
 }

+func (obsvbl *channelObservable[V]) Next(ctx context.Context) V {


Can you add a godoc on what this is or why its needed?

Added a godoc comment. It's not strictly necessary but will be convenient in forthcoming PRs.

It's not strictly necessary but will be convenient in forthcoming PRs.

I'm a bit confused as to when we should or shouldn't use it. #PUC when you have time, but not a blocker in this PR.

pkg/observable/interface.go

* pokt/main: chore: enforce go standard interface implementation registration (#87) [E2E] Add Regression Testing for Send E2E Feature Test (#84) feat: seperate tests from go_develop (#89) [Observable] chore: observable touchup (#83)

* chore: add `Observer#IsClosed()` to prevent redundant unsubscription (cherry picked from commit 78a9946b3f14353e79b123919416903d4622da4d) * chore: simplify channel observable (cherry picked from commit a2629c8bc3decfb5a787e453af67aa78fc8ca1ea) * test: add case for publisher w/ large buffer size, comment, & cleanup (cherry picked from commit e97b691e39af8fa1654b8d697a3b34095b32ed82) * docs: update observable pkg README.md (cherry picked from commit d5442c7062630d847e048850fa71806086f84172) * doc: fix pkg README template * chore: add `Observable#Next()` (cherry picked from commit cb4142f673fee37ead8520394e314f1fcb9d0dc9) * chore: update godoc comments

* Implement MsgStakeSupplier * Unit tests pass * Rename some test vars * Fix typo * Unstake - WIP * Update makefile * Finished implementing unstake * Self review * [WIP] Session Hydrator * snapshot commit * Working on the tests * One HydrateSessionTest ready * Self review - before finishing tests * Prepared templates for all of the unit tests * feat: add the map channel observable operator (cherry picked from commit 22371aa550eb0060b528f4573ba6908bbdfa0c1c) * Implemented TestSession_HydrateSession_Metadata * feat: add replay observable (cherry picked from commit ab21790164ab544ae5f1508d3237a3faab33e71e) * Implemented TestSession_HydrateSession_SessionId * chore: add query client interface * chore: add query client errors * Implemented TestSession_HydrateSession_Success_BaseCase * Implemented TestSession_HydrateSession_Application * Option attempt for tests * Implemented TestSession_HydrateSession_Suppliers * test: fix false positive, prevent regression, & add comments * chore: add godoc comment * feat: add query client implementation * chore: add connection & dialer wrapper implementations * test: query client & add testquery helper pkg * chore: add go_test_integration make target * chore: add internal mocks pkg * test: query client integration test * docs: add event query client docs * chore: update go.mod * chore: re-order `eventsQueryClient` methods to improve readability * chore: add godoc comments to testclient helpers * fix: comment formatting * chore: improve comment & naming in evt query client test * test: tune events query client parameters * chore: improve godoc comments * chore: review improvements * refactor: `replayObservable` as its own interface type * refactor: `replayObservable#Next() V` to `ReplayObservable#Last(ctx, n) []V` * chore: add constructor func for `ReplayObservable` * test: reorder to improve readibility * refactor: rename and add godoc comments * chore: improve naming & comments * chore: add warning log and improve comments * test: improve and add tests * fix: interface assertion * fix: comment typo * chore: review improvements * fix: race * chore: add block client interface * chore: add `MapReplay` operator * feat: add block client * test: block client integration * test: block client * docs: fix install instructions * fix: race on eventsBytesAndConns map * fix: interface assertions Co-authored-by: Redouane Lakrache <[email protected]> * fix: race * Apply suggestions from code review Co-authored-by: Bryan White <[email protected]> Co-authored-by: Redouane Lakrache <[email protected]> * [RelayerProxy] feat: implement relayerProxy struct (#82) * feat: add notifiable observable Co-authored-by: red-0ne <[email protected]> * fixup: observable (cherry picked from commit bcf700405b5e4bd71bf9bb650c988526fa16c728) * refactor/fix: notifiable observable improvements * chore: more review improvements * refactor: renaming - `notifiable` pkg to `channel` - `notifiableObservable` struct to `channelObservable` - `observer` struct to `channelObserver` - `notifier` vars to `producer` - `notifee` vars to `observable` (or similar) * chore: update comments * refactor: simplify drainCh test helper * test: fix timeout * test: rename observable test functions * test: add test TODOs * chore: update comments * refactor: simplify observable & observer * test: fix & add observable tests * test: cleanup & comment observable tests * fixup: observable (cherry picked from commit 33f3196535b7dae154e01f93aab36f70cda8fc4f) * fixup: observable test (cherry picked from commit 9c206da115dc35843d588313c2215a0e649c6df6) * refactor: simplify & cleanup * chore: cleanup logs & comments * chore: improve comments * refactor: DrainChannel test helper * shore: cleanup & simplify * test: comment out flaky test cases * fixup: drain channel helper * chore: improve var name * fixup: drain channel helper * test: shorten timeout * chore: cleanup * chore: cleanup, simplification, review improvements (cherry picked from commit 92a547da29ec526d415f6967ccfa5988c3f5ca1d) * chore: improve comments Co-authored-by: Daniel Olshansky <[email protected]> * chore: improve comments Co-authored-by: Daniel Olshansky <[email protected]> * refactor: rename `Observable#Close()` to `#UnsubscribeAll()` * chore: improve comments * chore: misc. review feedback improvements * chore: improve comment * chore: review improvements * chore: last minute improvements * feat: add RelayerProxy interface * Fix grammar in comments Co-authored-by: Daniel Olshansky <[email protected]> * chore: rename package to relayerproxy * feat: implement relayerProxy struct and its constructor args * fix: change directory structure * fix: change directory structure * chore: address change requests * chore: comment unavailable interface and its usage --------- Co-authored-by: Bryan White <[email protected]> Co-authored-by: Daniel Olshansky <[email protected]> * fix: RelayerProxy interface mismatch (#91) * [Observable] chore: observable touchup (#83) * chore: add `Observer#IsClosed()` to prevent redundant unsubscription (cherry picked from commit 78a9946b3f14353e79b123919416903d4622da4d) * chore: simplify channel observable (cherry picked from commit a2629c8bc3decfb5a787e453af67aa78fc8ca1ea) * test: add case for publisher w/ large buffer size, comment, & cleanup (cherry picked from commit e97b691e39af8fa1654b8d697a3b34095b32ed82) * docs: update observable pkg README.md (cherry picked from commit d5442c7062630d847e048850fa71806086f84172) * doc: fix pkg README template * chore: add `Observable#Next()` (cherry picked from commit cb4142f673fee37ead8520394e314f1fcb9d0dc9) * chore: update godoc comments * feat: seperate tests from go_develop (#89) * [E2E] Add Regression Testing for Send E2E Feature Test (#84) * chore: enforce go standard interface implementation registration (#87) * [Miner] feat: add the map channel observable operator (#92) * feat: add the map channel observable operator (cherry picked from commit 22371aa550eb0060b528f4573ba6908bbdfa0c1c) * test: fix false positive, prevent regression, & add comments * chore: add godoc comment * chore: review improvements * Reply to Red0ne's comments * Update small comment * Small self review * Updated TestSession_GetSession * Fixed last failing test * feat: add the interfaces for the RelayerSessions and SessionTree * chore: address change requests * chore: move-up comment * feat: add ExpiringSessions to RelaySessions interface * fix: use appropriate function name in comment Co-authored-by: Daniel Olshansky <[email protected]> * feat: implement relayerSessions and sessionTree * chore: improve comments * chore: address change requests * chore: remove alias types for sessionId and block height * chore: remove previous merge changes * chore: remove added gitkeep * chore: go mod tidy * fix: use conventional module paths * chore: address change request from PRs 104 & 105 * chore: wrap long comments * chore: address change requests --------- Co-authored-by: Daniel Olshansky <[email protected]> Co-authored-by: Bryan White <[email protected]> Co-authored-by: harry <[email protected]>

* chore: add `Observer#IsClosed()` to prevent redundant unsubscription (cherry picked from commit 78a9946b3f14353e79b123919416903d4622da4d) * chore: simplify channel observable (cherry picked from commit a2629c8bc3decfb5a787e453af67aa78fc8ca1ea) * test: add case for publisher w/ large buffer size, comment, & cleanup (cherry picked from commit e97b691e39af8fa1654b8d697a3b34095b32ed82) * docs: update observable pkg README.md (cherry picked from commit d5442c7062630d847e048850fa71806086f84172) * doc: fix pkg README template * chore: add `Observable#Next()` (cherry picked from commit cb4142f673fee37ead8520394e314f1fcb9d0dc9) * chore: update godoc comments

* Implement MsgStakeSupplier * Unit tests pass * Rename some test vars * Fix typo * Unstake - WIP * Update makefile * Finished implementing unstake * Self review * [WIP] Session Hydrator * snapshot commit * Working on the tests * One HydrateSessionTest ready * Self review - before finishing tests * Prepared templates for all of the unit tests * feat: add the map channel observable operator (cherry picked from commit 22371aa550eb0060b528f4573ba6908bbdfa0c1c) * Implemented TestSession_HydrateSession_Metadata * feat: add replay observable (cherry picked from commit ab21790164ab544ae5f1508d3237a3faab33e71e) * Implemented TestSession_HydrateSession_SessionId * chore: add query client interface * chore: add query client errors * Implemented TestSession_HydrateSession_Success_BaseCase * Implemented TestSession_HydrateSession_Application * Option attempt for tests * Implemented TestSession_HydrateSession_Suppliers * test: fix false positive, prevent regression, & add comments * chore: add godoc comment * feat: add query client implementation * chore: add connection & dialer wrapper implementations * test: query client & add testquery helper pkg * chore: add go_test_integration make target * chore: add internal mocks pkg * test: query client integration test * docs: add event query client docs * chore: update go.mod * chore: re-order `eventsQueryClient` methods to improve readability * chore: add godoc comments to testclient helpers * fix: comment formatting * chore: improve comment & naming in evt query client test * test: tune events query client parameters * chore: improve godoc comments * chore: review improvements * refactor: `replayObservable` as its own interface type * refactor: `replayObservable#Next() V` to `ReplayObservable#Last(ctx, n) []V` * chore: add constructor func for `ReplayObservable` * test: reorder to improve readibility * refactor: rename and add godoc comments * chore: improve naming & comments * chore: add warning log and improve comments * test: improve and add tests * fix: interface assertion * fix: comment typo * chore: review improvements * fix: race * chore: add block client interface * chore: add `MapReplay` operator * feat: add block client * test: block client integration * test: block client * docs: fix install instructions * fix: race on eventsBytesAndConns map * fix: interface assertions Co-authored-by: Redouane Lakrache <[email protected]> * fix: race * Apply suggestions from code review Co-authored-by: Bryan White <[email protected]> Co-authored-by: Redouane Lakrache <[email protected]> * [RelayerProxy] feat: implement relayerProxy struct (#82) * feat: add notifiable observable Co-authored-by: red-0ne <[email protected]> * fixup: observable (cherry picked from commit bcf700405b5e4bd71bf9bb650c988526fa16c728) * refactor/fix: notifiable observable improvements * chore: more review improvements * refactor: renaming - `notifiable` pkg to `channel` - `notifiableObservable` struct to `channelObservable` - `observer` struct to `channelObserver` - `notifier` vars to `producer` - `notifee` vars to `observable` (or similar) * chore: update comments * refactor: simplify drainCh test helper * test: fix timeout * test: rename observable test functions * test: add test TODOs * chore: update comments * refactor: simplify observable & observer * test: fix & add observable tests * test: cleanup & comment observable tests * fixup: observable (cherry picked from commit 33f3196535b7dae154e01f93aab36f70cda8fc4f) * fixup: observable test (cherry picked from commit 9c206da115dc35843d588313c2215a0e649c6df6) * refactor: simplify & cleanup * chore: cleanup logs & comments * chore: improve comments * refactor: DrainChannel test helper * shore: cleanup & simplify * test: comment out flaky test cases * fixup: drain channel helper * chore: improve var name * fixup: drain channel helper * test: shorten timeout * chore: cleanup * chore: cleanup, simplification, review improvements (cherry picked from commit 92a547da29ec526d415f6967ccfa5988c3f5ca1d) * chore: improve comments Co-authored-by: Daniel Olshansky <[email protected]> * chore: improve comments Co-authored-by: Daniel Olshansky <[email protected]> * refactor: rename `Observable#Close()` to `#UnsubscribeAll()` * chore: improve comments * chore: misc. review feedback improvements * chore: improve comment * chore: review improvements * chore: last minute improvements * feat: add RelayerProxy interface * Fix grammar in comments Co-authored-by: Daniel Olshansky <[email protected]> * chore: rename package to relayerproxy * feat: implement relayerProxy struct and its constructor args * fix: change directory structure * fix: change directory structure * chore: address change requests * chore: comment unavailable interface and its usage --------- Co-authored-by: Bryan White <[email protected]> Co-authored-by: Daniel Olshansky <[email protected]> * fix: RelayerProxy interface mismatch (#91) * [Observable] chore: observable touchup (#83) * chore: add `Observer#IsClosed()` to prevent redundant unsubscription (cherry picked from commit 78a9946b3f14353e79b123919416903d4622da4d) * chore: simplify channel observable (cherry picked from commit a2629c8bc3decfb5a787e453af67aa78fc8ca1ea) * test: add case for publisher w/ large buffer size, comment, & cleanup (cherry picked from commit e97b691e39af8fa1654b8d697a3b34095b32ed82) * docs: update observable pkg README.md (cherry picked from commit d5442c7062630d847e048850fa71806086f84172) * doc: fix pkg README template * chore: add `Observable#Next()` (cherry picked from commit cb4142f673fee37ead8520394e314f1fcb9d0dc9) * chore: update godoc comments * feat: seperate tests from go_develop (#89) * [E2E] Add Regression Testing for Send E2E Feature Test (#84) * chore: enforce go standard interface implementation registration (#87) * [Miner] feat: add the map channel observable operator (#92) * feat: add the map channel observable operator (cherry picked from commit 22371aa550eb0060b528f4573ba6908bbdfa0c1c) * test: fix false positive, prevent regression, & add comments * chore: add godoc comment * chore: review improvements * Reply to Red0ne's comments * Update small comment * Small self review * Updated TestSession_GetSession * Fixed last failing test * feat: add the interfaces for the RelayerSessions and SessionTree * chore: address change requests * chore: move-up comment * feat: add ExpiringSessions to RelaySessions interface * fix: use appropriate function name in comment Co-authored-by: Daniel Olshansky <[email protected]> * feat: implement relayerSessions and sessionTree * chore: improve comments * chore: address change requests * chore: remove alias types for sessionId and block height * chore: remove previous merge changes * chore: remove added gitkeep * chore: go mod tidy * fix: use conventional module paths * chore: address change request from PRs 104 & 105 * chore: wrap long comments * chore: address change requests --------- Co-authored-by: Daniel Olshansky <[email protected]> Co-authored-by: Bryan White <[email protected]> Co-authored-by: harry <[email protected]>

bryanchriswhite added the off-chain Off-chain business logic label Oct 20, 2023

bryanchriswhite added this to the Shannon TestNet milestone Oct 20, 2023

bryanchriswhite self-assigned this Oct 20, 2023

bryanchriswhite added 5 commits October 23, 2023 17:41

chore: add Observer#IsClosed() to prevent redundant unsubscription

ad77da4

(cherry picked from commit 78a9946b3f14353e79b123919416903d4622da4d)

chore: simplify channel observable

f88b070

(cherry picked from commit a2629c8bc3decfb5a787e453af67aa78fc8ca1ea)

test: add case for publisher w/ large buffer size, comment, & cleanup

dfb2aae

(cherry picked from commit e97b691e39af8fa1654b8d697a3b34095b32ed82)

docs: update observable pkg README.md

0ea010a

(cherry picked from commit d5442c7062630d847e048850fa71806086f84172)

doc: fix pkg README template

17b0ce5

bryanchriswhite force-pushed the chore/observable-touchup branch from 1042925 to 17b0ce5 Compare October 23, 2023 15:42

bryanchriswhite marked this pull request as ready for review October 23, 2023 15:59

bryanchriswhite requested review from Olshansk and red-0ne October 23, 2023 16:00

bryanchriswhite added 2 commits October 23, 2023 18:07

chore: add Observable#Next()

ebb30f9

(cherry picked from commit cb4142f673fee37ead8520394e314f1fcb9d0dc9)

Merge remote-tracking branch 'pokt/main' into chore/observable-touchup

d90c3d7

* pokt/main: fix: RelayerProxy interface mismatch (#91)

Olshansk requested changes Oct 23, 2023

View reviewed changes

chore: update godoc comments

1cca119

bryanchriswhite requested a review from Olshansk October 23, 2023 16:27

Olshansk approved these changes Oct 23, 2023

View reviewed changes

bryanchriswhite merged commit e7c5e5f into main Oct 23, 2023
6 checks passed

bryanchriswhite deleted the chore/observable-touchup branch October 23, 2023 16:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Observable] chore: observable touchup #83

[Observable] chore: observable touchup #83

bryanchriswhite commented Oct 20, 2023 •

edited by reviewpad bot

Loading

bryanchriswhite commented Oct 23, 2023

Olshansk Oct 23, 2023

Olshansk Oct 23, 2023

bryanchriswhite Oct 23, 2023

Olshansk Oct 23, 2023

Olshansk Oct 23, 2023

bryanchriswhite Oct 23, 2023

Olshansk Oct 23, 2023

[Observable] chore: observable touchup #83

[Observable] chore: observable touchup #83

Conversation

bryanchriswhite commented Oct 20, 2023 • edited by reviewpad bot Loading

Summary

Human Summary

AI Summary

Summary generated by Reviewpad on 23 Oct 23 16:25 UTC

Issue

Type of change

Testing

Sanity Checklist

bryanchriswhite commented Oct 23, 2023

Olshansk Oct 23, 2023

Choose a reason for hiding this comment

Olshansk Oct 23, 2023

Choose a reason for hiding this comment

bryanchriswhite Oct 23, 2023

Choose a reason for hiding this comment

Olshansk Oct 23, 2023

Choose a reason for hiding this comment

Olshansk Oct 23, 2023

Choose a reason for hiding this comment

bryanchriswhite Oct 23, 2023

Choose a reason for hiding this comment

Olshansk Oct 23, 2023

Choose a reason for hiding this comment

bryanchriswhite commented Oct 20, 2023 •

edited by reviewpad bot

Loading