Skip to content

backport: set a minimum long-polling interval for Events in rpc (#8050)#9768

Merged
thanethomson merged 2 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-pollways
Dec 7, 2022
Merged

backport: set a minimum long-polling interval for Events in rpc (#8050)#9768
thanethomson merged 2 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-pollways

Conversation

@mmsqe
Copy link

@mmsqe mmsqe commented Nov 26, 2022

For the full description, see #8050


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

Since the goal of reading events at the head of the event log is to satisfy a
subscription style interface, there is no point in allowing head polling with
no wait interval. The pagination case already bypasses long polling, so the
extra option is unneessary.

Set a minimum default long-polling interval for the head case.
Add a test for minimum delay.
@mmsqe mmsqe marked this pull request as ready for review November 26, 2022 01:19
@mmsqe mmsqe requested a review from ebuchman as a code owner November 26, 2022 01:19
@mmsqe mmsqe requested a review from a team November 26, 2022 01:19
@github-actions
Copy link

github-actions bot commented Dec 7, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale for use by stalebot label Dec 7, 2022
maxItems = 100
}

const minWaitTime = 1 * time.Second
Copy link
Author

Choose a reason for hiding this comment

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

@cmwaters @sergio-mena feel to ping me if any change is needed

@thanethomson thanethomson added S:wip Work in progress (prevents stalebot from automatically closing) and removed stale for use by stalebot labels Dec 7, 2022
Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thanethomson thanethomson merged commit 1b1ba41 into tendermint:feature/adr075-backport Dec 7, 2022
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.

Sorry for delay on this.
LGTM (thanks @thanethomson)

mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
…ermint#8050) (tendermint#9768)

* rpc: set a minimum long-polling interval for Events (tendermint#8050)

Since the goal of reading events at the head of the event log is to satisfy a
subscription style interface, there is no point in allowing head polling with
no wait interval. The pagination case already bypasses long polling, so the
extra option is unneessary.

Set a minimum default long-polling interval for the head case.
Add a test for minimum delay.

* fix doc

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

* rpc: set a minimum long-polling interval for Events (tendermint#8050)

Since the goal of reading events at the head of the event log is to satisfy a
subscription style interface, there is no point in allowing head polling with
no wait interval. The pagination case already bypasses long polling, so the
extra option is unneessary.

Set a minimum default long-polling interval for the head case.
Add a test for minimum delay.

* fix doc

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

S:wip Work in progress (prevents stalebot from automatically closing)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants