Skip to content

backport: rewrite the WaitForOneEvent helper for rpc/client (#7986)#9550

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

backport: rewrite the WaitForOneEvent helper for rpc/client (#7986)#9550
sergio-mena merged 1 commit intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-helper

Conversation

@mmsqe
Copy link

@mmsqe mmsqe commented Oct 14, 2022

For the full description, see #7986


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

@mmsqe mmsqe marked this pull request as ready for review October 14, 2022 01:23
@mmsqe mmsqe requested a review from a team October 14, 2022 01:23
@github-actions
Copy link

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 Oct 25, 2022
@mmsqe
Copy link
Author

mmsqe commented Oct 25, 2022

@cmwaters @sergio-mena May I know if any change is needed here?

if err != nil {
return err
} else if len(rsp.Items) == 0 {
continue // continue polling until ctx expires
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it would be better to wait between polling but I guess it's fine

Copy link
Author

Choose a reason for hiding this comment

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

Actually WaitForOneEvent keep continue since err set to nil when timeout.

Copy link
Author

Choose a reason for hiding this comment

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

Do we need further change? Since rpc doesn't report a timeout as a request failure and this function seems only use for test.

Copy link
Contributor

Choose a reason for hiding this comment

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

No I think it's okay. As you said, this is just used for testing

@github-actions github-actions bot removed the stale for use by stalebot label Oct 26, 2022
@sergio-mena sergio-mena merged commit d6d3c17 into tendermint:feature/adr075-backport Nov 3, 2022
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
…dermint#9550)

Update usage in tests.

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#9550)

Update usage in tests.

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