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

feat(sync-v2): Add both BlockchainStreamingClient and TransactionStreamingClient to manage streamings from the client side #848

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Nov 3, 2023

Motivation

Even though streaming servers were implemented on separated classes, the clients were intertwined within the sync agent.

Acceptance Criteria

  1. Add BlockchainStreamingClient to handle blockchain streamings.
  2. Add TransactionStreamingClient to handle transaction streamings.
  3. Rename until_first_block to last_block_hash in GET-TRANSACTION-BFS message.
  4. Add first_block_hash parameter to GET-TRANSACTION-BFS message.
  5. Besides the changes in the GET-TRANSACTION-BFS, the changes in this PR can be considered a refactor and no other behavior should be changed.
  6. Add pydantic models to GET-NEXT-BLOCKS, BEST-BLOCK and GET-TRANSACTIONS-BFS payloads.

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 3, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner November 3, 2023 05:27
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch 4 times, most recently from 0b93ea0 to b9b06e8 Compare November 3, 2023 07:07
@msbrogli msbrogli changed the title Feat/sync v2 improvements 3 feat(sync-v2): Add both BlockchainStreamingClient and TransactionStreamingClient to manage streamings from the client side Nov 3, 2023
hathor/p2p/sync_v2/agent.py Outdated Show resolved Hide resolved
@msbrogli msbrogli requested a review from glevco November 3, 2023 07:25
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from a340ba6 to de60b5c Compare November 3, 2023 08:30
hathor/p2p/sync_v2/agent.py Show resolved Hide resolved
hathor/p2p/sync_v2/agent.py Outdated Show resolved Hide resolved
hathor/p2p/sync_v2/payloads.py Show resolved Hide resolved
hathor/p2p/sync_v2/payloads.py Outdated Show resolved Hide resolved
hathor/p2p/sync_v2/payloads.py Outdated Show resolved Hide resolved
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-2 branch from 708b5a8 to 38158b6 Compare November 3, 2023 16:24
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from de60b5c to e37b28a Compare November 3, 2023 16:25
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-2 branch from 38158b6 to 92bde24 Compare November 3, 2023 17:51
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from e37b28a to 952a22d Compare November 3, 2023 17:52
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-2 branch from 92bde24 to 4b04c9c Compare November 3, 2023 18:07
Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Attention: 47 lines in your changes are missing coverage. Please review.

Comparison is base (4b04c9c) 85.01% compared to head (5b59091) 85.06%.

❗ Current head 5b59091 differs from pull request most recent head eab8986. Consider uploading reports for the commit eab8986 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #848      +/-   ##
==========================================
+ Coverage   85.01%   85.06%   +0.05%     
==========================================
  Files         271      275       +4     
  Lines       22389    22502     +113     
  Branches     3417     3434      +17     
==========================================
+ Hits        19034    19142     +108     
+ Misses       2678     2674       -4     
- Partials      677      686       +9     
Files Coverage Δ
hathor/p2p/sync_v2/exception.py 100.00% <100.00%> (ø)
hathor/p2p/sync_v2/streamers.py 90.05% <100.00%> (+0.05%) ⬆️
hathor/p2p/sync_v2/payloads.py 94.44% <94.44%> (ø)
hathor/p2p/sync_v2/agent.py 78.42% <83.78%> (+2.89%) ⬆️
hathor/p2p/sync_v2/transaction_streaming_client.py 75.40% <75.40%> (ø)
hathor/p2p/sync_v2/blockchain_streaming_client.py 76.62% <76.62%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from 952a22d to 6d9b085 Compare November 3, 2023 18:25
jansegre
jansegre previously approved these changes Nov 3, 2023
Base automatically changed from feat/sync-v2-improvements-2 to master November 3, 2023 19:11
@msbrogli msbrogli dismissed jansegre’s stale review November 3, 2023 19:11

The base branch was changed.

jansegre
jansegre previously approved these changes Nov 3, 2023
glevco
glevco previously approved these changes Nov 3, 2023
hathor/p2p/sync_v2/exception.py Outdated Show resolved Hide resolved
hathor/p2p/sync_v2/payloads.py Outdated Show resolved Hide resolved
…amingClient to manage streamings from the client side
@msbrogli msbrogli force-pushed the feat/sync-v2-improvements-3 branch from 6dd0b59 to eab8986 Compare November 3, 2023 21:06
@msbrogli msbrogli merged commit eab8986 into master Nov 3, 2023
@msbrogli msbrogli deleted the feat/sync-v2-improvements-3 branch November 3, 2023 21:06
@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