Skip to content

backport: add eventstream helper for rpc/client (#7987)#9659

Merged
sergio-mena merged 1 commit intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-stream
Nov 17, 2022
Merged

backport: add eventstream helper for rpc/client (#7987)#9659
sergio-mena merged 1 commit intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-stream

Conversation

@mmsqe
Copy link

@mmsqe mmsqe commented Nov 3, 2022

For the full description, see #7987


PR checklist

  • Tests written/updated, or no tests needed
  • CHANGELOG_PENDING.md updated, or no changelog entry needed
  • Updated relevant documentation (docs/) and code comments, or no
    documentation updates needed

This allows the caller to stream events. It handles the bookkeeping for cursors
and pagination, and delivers items to a callback.

Handle missed items by reporting a structured error. The caller can use the
Reset method to "catch up" to head after this happens.

Add a manual test CLI to probe a running node. Requires the node to be
configured with the event log settings.

Add a unit test that scripts input to the stream to exercise it.
@mmsqe mmsqe marked this pull request as ready for review November 3, 2022 09:39
@mmsqe mmsqe requested a review from a team November 3, 2022 09:39
@sergio-mena
Copy link
Contributor

Will review soon (sorry for delay)

Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

LGTM

func makeTestItem(cur, data string) testItem {
return testItem{
Cursor: cur,
Data: fmt.Sprintf(`%q`, data),
Copy link
Contributor

Choose a reason for hiding this comment

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

The original Sprintf formatting is:

`{"type":%q,"value":%q}`, types.EventDataString("").TypeTag(), data

Any reason why you simplified it?

Copy link
Author

Choose a reason for hiding this comment

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

To adjust json.Marshal while origin could make use of jsontypes.Marshal

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, since #7600 hasn't been backported, I believe.
If that is the case, I'll approve.
In any case, do you think if would be beneficial to backport #7600 in a separate PR?

Copy link
Author

Choose a reason for hiding this comment

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

I think the tradeoff is mentioned before, and only 3 small PRs left for this issue.

Copy link
Contributor

@sergio-mena sergio-mena 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 your patience!

@sergio-mena sergio-mena merged commit 892c765 into tendermint:feature/adr075-backport Nov 17, 2022
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
This allows the caller to stream events. It handles the bookkeeping for cursors
and pagination, and delivers items to a callback.

Handle missed items by reporting a structured error. The caller can use the
Reset method to "catch up" to head after this happens.

Add a manual test CLI to probe a running node. Requires the node to be
configured with the event log settings.

Add a unit test that scripts input to the stream to exercise it.

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
This allows the caller to stream events. It handles the bookkeeping for cursors
and pagination, and delivers items to a callback.

Handle missed items by reporting a structured error. The caller can use the
Reset method to "catch up" to head after this happens.

Add a manual test CLI to probe a running node. Requires the node to be
configured with the event log settings.

Add a unit test that scripts input to the stream to exercise it.

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
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.

3 participants