Skip to content

refactor(p2p): Build ConnectionsManager in builder#622

Merged
msbrogli merged 1 commit intomasterfrom
refactor/builder-p2p-manager
May 24, 2023
Merged

refactor(p2p): Build ConnectionsManager in builder#622
msbrogli merged 1 commit intomasterfrom
refactor/builder-p2p-manager

Conversation

@msbrogli
Copy link
Member

@msbrogli msbrogli commented May 19, 2023

Acceptance criteria

  1. Build ConnectionsManager in builders.
  2. Move ssl: bool, enable_sync_v1, enable_sync_v1_1, and enable_sync_v2 to ConnectionsManager.
  3. Remove HathorManager.server_factory and HathorManager.client_factory.

Note

There are still some network-related attributes and methods on HathorManager. I'll move those on a separate refactor to keep this one small for reviewers. Some attributes and methods yet-to-be moved out of HathorManager are:

  • listen_addresses: List[str]
  • capabilities
  • has_sync_version_capability()
  • add_peer_to_whitelist()
  • remove_peer_from_whitelist_and_disconnect()
  • listen()
  • do_discovery()

@msbrogli msbrogli self-assigned this May 19, 2023
@msbrogli msbrogli requested a review from jansegre as a code owner May 19, 2023 23:05
@msbrogli msbrogli force-pushed the refactor/builder-p2p-manager branch 8 times, most recently from 743f4d9 to 6d1fe41 Compare May 20, 2023 01:33
@codecov
Copy link

codecov bot commented May 20, 2023

Codecov Report

Merging #622 (6d1fe41) into master (c5c827e) will decrease coverage by 0.12%.
The diff coverage is 93.10%.

❗ Current head 6d1fe41 differs from pull request most recent head f822d88. Consider uploading reports for the commit f822d88 to get more accurate results

@@            Coverage Diff             @@
##           master     #622      +/-   ##
==========================================
- Coverage   83.74%   83.63%   -0.12%     
==========================================
  Files         236      232       -4     
  Lines       19949    19857      -92     
  Branches     2727     2712      -15     
==========================================
- Hits        16707    16608      -99     
- Misses       2643     2650       +7     
  Partials      599      599              
Impacted Files Coverage Δ
hathor/builder/builder.py 89.75% <80.76%> (-1.37%) ⬇️
hathor/p2p/manager.py 72.51% <95.83%> (+2.14%) ⬆️
hathor/builder/cli_builder.py 71.05% <100.00%> (+0.78%) ⬆️
hathor/manager.py 70.08% <100.00%> (-0.52%) ⬇️
hathor/p2p/factory.py 100.00% <100.00%> (ø)
hathor/p2p/protocol.py 92.98% <100.00%> (-0.06%) ⬇️
hathor/p2p/sync_v1_1_factory.py 100.00% <100.00%> (ø)
hathor/p2p/sync_v1_factory.py 100.00% <100.00%> (ø)
hathor/simulator/fake_connection.py 82.73% <100.00%> (ø)

... and 16 files with indirect coverage changes

@msbrogli msbrogli requested a review from glevco May 20, 2023 02:08
glevco
glevco previously approved these changes May 22, 2023
jansegre
jansegre previously approved these changes May 22, 2023
@msbrogli msbrogli dismissed stale reviews from jansegre and glevco via f822d88 May 24, 2023 14:32
@msbrogli msbrogli force-pushed the refactor/builder-p2p-manager branch from 6d1fe41 to f822d88 Compare May 24, 2023 14:32
@msbrogli msbrogli merged commit f822d88 into master May 24, 2023
@msbrogli msbrogli deleted the refactor/builder-p2p-manager branch May 24, 2023 14:35
@jansegre jansegre mentioned this pull request Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants