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

[Off-chain] feat: observable utils #171

Merged
merged 10 commits into from
Nov 9, 2023

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Nov 9, 2023

Summary

Adds:

  • ForEach() calls the given function for each notification (like Map() without an destination)
  • Filters:
    • EitherError() filters out only eithers with error populated and maps the error to the destination
    • EitherSuccess() filters out only eithers with value populated and maps the value to the destination
  • LogErrors() logs notifications from the given error observable

Human Summary

AI Summary

Summary generated by Reviewpad on 09 Nov 23 19:54 UTC

This pull request includes three changes:

  1. Added a new file pkg/observable/filter/either.go, which provides functions for filtering and mapping an observable of an either type. It includes functions EitherError and EitherSuccess for filtering eithers based on whether they are populated with errors or values, respectively.

  2. Added a new file pkg/observable/logging/logging.go, which includes functions for logging errors received from an observable. The LogErrors function logs all errors received from the observable.

  3. Modified the existing file pkg/observable/channel/map.go to include a new type ForEachFn and a new function ForEach. The ForEach function applies the given forEachFn to each notification received from the observable, similar to Map, but without publishing to a destination observable. It is useful for side effects and is a terminal observable operator.

Issue

  • #{ISSUE_NUMBER}

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

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make go_develop_and_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

@bryanchriswhite bryanchriswhite self-assigned this Nov 9, 2023
@bryanchriswhite bryanchriswhite added miner Changes related to the Miner off-chain Off-chain business logic labels Nov 9, 2023
@bryanchriswhite bryanchriswhite added this to the Shannon TestNet milestone Nov 9, 2023
@bryanchriswhite bryanchriswhite marked this pull request as ready for review November 9, 2023 15:59
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

Couple comments but very nice and clean!

pkg/observable/channel/map.go Outdated Show resolved Hide resolved
pkg/observable/channel/map.go Outdated Show resolved Hide resolved
pkg/observable/channel/map.go Outdated Show resolved Hide resolved
return value, true
}
return value, false
}
Copy link
Member

Choose a reason for hiding this comment

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

Great comments, naming, design, and everything across the board

@bryanchriswhite bryanchriswhite changed the base branch from issues/13/refactor/map to main November 9, 2023 19:54
@bryanchriswhite bryanchriswhite merged commit 3e18472 into main Nov 9, 2023
3 checks passed
@bryanchriswhite bryanchriswhite deleted the issues/13/feat/observable-utils branch November 9, 2023 20:14
bryanchriswhite added a commit that referenced this pull request Nov 9, 2023
* pokt/main:
  [Off-chain] feat: observable utils (#171)
  [Off-chain] refactor: `MapFn`s receive context arg (#170)
okdas pushed a commit that referenced this pull request Nov 14, 2024
* refactor: `MapFn`s receive context arg

* chore: add `ForEach` map shorthand operator

* chore: add `/pkg/observable/filter`

* chore: add `/pkg/observable/logging`

* chore: add godoc comments

* chore: review feedback improvement

* fix: `MapReplay` usages post-refactor

* chore: review feedback improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
miner Changes related to the Miner off-chain Off-chain business logic
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants