Skip to content

backport: implement the ADR 075 /events method (#7965)#9497

Merged
sergio-mena merged 3 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-service
Oct 4, 2022
Merged

backport: implement the ADR 075 /events method (#7965)#9497
sergio-mena merged 3 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-service

Conversation

@mmsqe
Copy link

@mmsqe mmsqe commented Sep 28, 2022

For the full description, see #7965


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

M. J. Fromberger and others added 2 commits September 28, 2022 22:40
This method implements the eventlog extension interface to expose ABCI metadata
to the log for query processing. Only the types that have ABCI events need to
implement this.

- Add an event log to the environment
- Add a sketch of the handler method
- Add an /events RPCFunc to the route map
- Implement query logic
- Subscribe to pubsub if confingured, handle termination
@mmsqe mmsqe force-pushed the feature/adr075-backport-service branch from 5f235bd to 7a40bd5 Compare September 28, 2022 14:42
@mmsqe mmsqe marked this pull request as ready for review September 28, 2022 14:53
@mmsqe mmsqe requested a review from ebuchman as a code owner September 28, 2022 14:53
@mmsqe mmsqe requested a review from a team September 28, 2022 14:53
return q.matchesEvents(ExpandEvents(events)), nil
}

func (q *Query) MatchesEvents(events []types.Event) (bool, error) {
Copy link
Author

Choose a reason for hiding this comment

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

@sergio-mena MatchesEvents is added to avoid bigger diff when apply origin Matches, let me know if need changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems fine to me

Copy link
Contributor

Choose a reason for hiding this comment

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

I also find this OK.
I have a question though: is adding MatchesEvents a temporary way to reduce diff size? Or will it stay when all PRs needed are backported to his branch?

@sergio-mena
Copy link
Contributor

@mmsqe are all UTs passing on your machine?

@mmsqe
Copy link
Author

mmsqe commented Oct 4, 2022

@mmsqe are all UTs passing on your machine?

Do you mean pass when run make test?

@sergio-mena
Copy link
Contributor

@mmsqe are all UTs passing on your machine?

Do you mean pass when run make test?

Yes, sorry should have been clearer

@mmsqe
Copy link
Author

mmsqe commented Oct 4, 2022

@mmsqe are all UTs passing on your machine?

Do you mean pass when run make test?

Yes, sorry should have been clearer

Tests are green, ci also says so.


// Verify that the event data types satisfy their shared interface.
var (
_ TMEventData = EventDataCompleteProposal{}
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 EventDataBlockSyncStatus and EventDataStateSyncStatus are not used here.
Rightly so, as they are not defined in this branch.
Would it make sense to backport to this feature branch #6755 and/or #6700, which defined then?

Copy link
Author

Choose a reason for hiding this comment

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

maybe we can add TODO here to avoid miss when backport these feature branches later

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the TODO, I think it's a good idea.
My question was rather, whether THIS is the right moment to bring on #6755 (and any other it depends on), and #6700.

Copy link
Author

Choose a reason for hiding this comment

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

I could help to backport these 2 PRs later, not sure if we give them a priority.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough. If you could help with it (backporting them in this branch), it would be awesome

@sergio-mena sergio-mena merged commit d47b675 into tendermint:feature/adr075-backport Oct 4, 2022
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
…dermint#9497)

* rpc: implement the ADR 075 /events method (tendermint#7965)

This method implements the eventlog extension interface to expose ABCI metadata
to the log for query processing. Only the types that have ABCI events need to
implement this.

- Add an event log to the environment
- Add a sketch of the handler method
- Add an /events RPCFunc to the route map
- Implement query logic
- Subscribe to pubsub if confingured, handle termination

* add MatchesEvents test

* add TODO due to backport sequence

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

* rpc: implement the ADR 075 /events method (tendermint#7965)

This method implements the eventlog extension interface to expose ABCI metadata
to the log for query processing. Only the types that have ABCI events need to
implement this.

- Add an event log to the environment
- Add a sketch of the handler method
- Add an /events RPCFunc to the route map
- Implement query logic
- Subscribe to pubsub if confingured, handle termination

* add MatchesEvents test

* add TODO due to backport sequence

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