Skip to content

Feature/adr075 backport eventlog#9470

Merged
sergio-mena merged 2 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-eventlog
Sep 23, 2022
Merged

Feature/adr075 backport eventlog#9470
sergio-mena merged 2 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-eventlog

Conversation

@mmsqe
Copy link

@mmsqe mmsqe commented Sep 21, 2022

See #7825 for a full description


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 21, 2022 20:33
Implement the basic cursor and eventlog types described in ADR 075.  Handle
encoding and decoding as strings for compatibility with JSON.

- Add unit tests for the required order and synchronization properties.
- Add hooks for metrics, with one value to be expanded later.
- Update ADR 075 to match the specifics of the implementation so far.
@mmsqe mmsqe marked this pull request as ready for review September 21, 2022 13:10
@mmsqe mmsqe requested a review from ebuchman as a code owner September 21, 2022 13:10
@mmsqe mmsqe requested a review from a team September 21, 2022 13:10
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.

From a purely back-porting perspective, this LGTM

items to arrive at the head of the log. If `wait_time` is zero or negative, the
server will wait for a default (positive) interval.
items to arrive at the head of the log. If `wait_time` is zero, the server will
return whatever eligible items are available immediately.
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 PR had many hunks not present here. I went through them and they are all included already (probably because the ADRs on main were copied from master.
So all good here.

//
// Any error reported by Add arises from pruning; the new item was added to the
// log regardless whether an error occurs.
func (lg *Log) Add(etype string, data types.TMEventData) error {
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 PR uses type.EventData.
I see this was introduced by #7687. In that PR, besides renaming TMEventData to EventData, the interface was also extended with a field.
Is this relevant or necessary for this PR? Would there be a benefit in backporting #7687 prior to this one?

Copy link
Author

Choose a reason for hiding this comment

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

just try keep backport as minimal as possible since #7687 require jsontypes introduced in #7600

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the answer.
I see there is a series of "rpc cleanup" PRs by Michael that were merged to v0.36.x (and are therefore not in main or feature/adr075-backport). We might want to make sure ADR075-related PRs to backport are not assuming functionality introduced/changed/improved by those cleanup PRs.

@mmsqe mmsqe requested a review from sergio-mena September 23, 2022 09:08
@sergio-mena sergio-mena merged commit f1a57ad into tendermint:feature/adr075-backport Sep 23, 2022
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
* rpc: implement the eventlog defined by ADR 075 (tendermint#7825)

Implement the basic cursor and eventlog types described in ADR 075.  Handle
encoding and decoding as strings for compatibility with JSON.

- Add unit tests for the required order and synchronization properties.
- Add hooks for metrics, with one value to be expanded later.
- Update ADR 075 to match the specifics of the implementation so far.

* fix event type

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
* rpc: implement the eventlog defined by ADR 075 (tendermint#7825)

Implement the basic cursor and eventlog types described in ADR 075.  Handle
encoding and decoding as strings for compatibility with JSON.

- Add unit tests for the required order and synchronization properties.
- Add hooks for metrics, with one value to be expanded later.
- Update ADR 075 to match the specifics of the implementation so far.

* fix event type

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.

2 participants