Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sync-v2): Fix stream end messages for both blockchains and transactions #870

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Nov 10, 2023

Motivation

Stream servers were being stopped in multiple places but sometimes without sending an end message or sending the wrong type of message.

Acceptance Criteria

  1. Add StreamEnd.PER_REQUEST.
  2. Introduce both stop_tx_streaming_server() and stop_blk_streaming_server() methods.
  3. Update previous calls to send_transactions_end() to stop_tx_streaming_server().
  4. Update previous calls to send_blocks_end() to stop_blk_streaming_server().
  5. Fix internal errors in streamers always sending BLOCKS-END messages.
  6. Fix handle_get_transactions_bfs() wrongfully sending BLOCKS-END message.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@msbrogli msbrogli self-assigned this Nov 10, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner November 10, 2023 23:06
@msbrogli msbrogli requested a review from glevco November 10, 2023 23:12
@msbrogli msbrogli force-pushed the fix/sync-v2-streams-end branch from 95830d5 to 090bd71 Compare November 10, 2023 23:30
@msbrogli msbrogli merged commit 090bd71 into master Nov 10, 2023
@msbrogli msbrogli deleted the fix/sync-v2-streams-end branch November 10, 2023 23:31
@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants