Skip to content

backport: add Events method to the client interface (#7982)#9519

Merged
sergio-mena merged 2 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-client
Oct 13, 2022
Merged

backport: add Events method to the client interface (#7982)#9519
sergio-mena merged 2 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-client

Conversation

@mmsqe
Copy link

@mmsqe mmsqe commented Oct 7, 2022

For the full description, see #7982


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

- Update documentation to deprecate the old methods.
- Add Events methods to HTTP, WS, and Local clients.
- Add Events method to the light client wrapper.
- Rename legacy events client to SubscriptionClient.
@mmsqe mmsqe marked this pull request as ready for review October 7, 2022 01:27
@mmsqe mmsqe requested a review from a team October 7, 2022 01:27
waitTime time.Duration,
) (*ctypes.ResultEvents, error)

func makeEventsSearchFunc(c *lrpc.Client) rpcEventsSearchFunc {
Copy link
Author

Choose a reason for hiding this comment

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

I need apply change to adapt non-primitive params without pattern change, since we can't get struct like filter or cursor directly from route without json parser refactor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know which PR the json parser refactor was?

Copy link
Author

Choose a reason for hiding this comment

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

Mainly miss parseParams.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, thanks. Noted it as an issue

}

// TODO(creachadair): This interface should be removed once the streaming event
// interface is removed in Tendermint v0.37.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// interface is removed in Tendermint v0.37.
// interface is removed in Tendermint v0.39.

nit

waitTime time.Duration,
) (*ctypes.ResultEvents, error)

func makeEventsSearchFunc(c *lrpc.Client) rpcEventsSearchFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know which PR the json parser refactor was?

@mmsqe mmsqe requested a review from cmwaters October 12, 2022 01:01
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.

LGTM

}

func (c *Local) Events(ctx context.Context, req *ctypes.RequestEvents) (*ctypes.ResultEvents, error) {
return core.Events(c.ctx, req.Filter.Query, req.MaxItems, req.Before, req.After, req.WaitTime)
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity: #7982 implements the new functionality directly in this file, whereas this PR moves it inside the core package. Was there a reason?

Copy link
Author

Choose a reason for hiding this comment

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

To allow reuse in here and routes, while #7982 with parseParams refactor only use in local.

@sergio-mena sergio-mena merged commit 96dd4d0 into tendermint:feature/adr075-backport Oct 13, 2022
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
…tendermint#9519)

* rpc/client: add Events method to the client interface (tendermint#7982)

- Update documentation to deprecate the old methods.
- Add Events methods to HTTP, WS, and Local clients.
- Add Events method to the light client wrapper.
- Rename legacy events client to SubscriptionClient.

* Apply suggestions from code review

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
…tendermint#9519)

* rpc/client: add Events method to the client interface (tendermint#7982)

- Update documentation to deprecate the old methods.
- Add Events methods to HTTP, WS, and Local clients.
- Add Events method to the light client wrapper.
- Rename legacy events client to SubscriptionClient.

* Apply suggestions from code review

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