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: Update solo to load remote config near entry point #1176

Merged

Conversation

Ivo-Yankov
Copy link
Contributor

@Ivo-Yankov Ivo-Yankov commented Jan 17, 2025

Description

  • Removes all usages of RemoteConfigTasks.loadRemoteConfig.bind(this)(argv),
  • Adds a new middleware that loads the remote config before the commands are executed. This is skipped for several commands:
    init, node keys, cluster info, cluster list - They do not require connecting to a remote cluster.
    deployment create, cluster connect - Require connecting to several clusters and that is handled by the command task logic.

Related Issues

Copy link
Contributor

Unit Test Results - Linux

  1 files  ±0   59 suites  ±0   3s ⏱️ ±0s
233 tests ±0  233 ✅ ±0  0 💤 ±0  0 ❌ ±0 
238 runs  ±0  238 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b2df521. ± Comparison against base commit ab018a7.

Copy link
Contributor

Unit Test Results - Windows

  1 files  ±0   59 suites  ±0   16s ⏱️ ±0s
233 tests ±0  233 ✅ ±0  0 💤 ±0  0 ❌ ±0 
238 runs  ±0  238 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b2df521. ± Comparison against base commit ab018a7.

@Ivo-Yankov Ivo-Yankov self-assigned this Jan 17, 2025
@Ivo-Yankov Ivo-Yankov marked this pull request as ready for review January 17, 2025 15:19
@Ivo-Yankov Ivo-Yankov requested review from leninmehedy and a team as code owners January 17, 2025 15:19
@Ivo-Yankov Ivo-Yankov added the PR: Needs Approval A pull request that needs reviews and approvals. label Jan 17, 2025
Copy link
Contributor

E2E Test Report

 17 files  ±0  126 suites  ±0   1h 26m 23s ⏱️ +17s
258 tests ±0  258 ✅ ±0  0 💤 ±0  0 ❌ ±0 
269 runs  ±0  269 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b2df521. ± Comparison against base commit ab018a7.

1 similar comment
Copy link
Contributor

E2E Test Report

 17 files  ±0  126 suites  ±0   1h 26m 23s ⏱️ +17s
258 tests ±0  258 ✅ ±0  0 💤 ±0  0 ❌ ±0 
269 runs  ±0  269 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b2df521. ± Comparison against base commit ab018a7.

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.12% (target: -1.00%) 29.41%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (ab018a7) 21308 17829 83.67%
Head commit (b2df521) 21320 (+12) 17814 (-15) 83.56% (-0.12%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#1176) 34 10 29.41%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

Attention: Patch coverage is 29.41176% with 24 lines in your changes missing coverage. Please review.

Project coverage is 82.76%. Comparing base (ab018a7) to head (b2df521).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/core/config/remote/remote_config_manager.ts 27.27% 24 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1176      +/-   ##
==========================================
- Coverage   82.91%   82.76%   -0.16%     
==========================================
  Files          77       77              
  Lines       21335    21320      -15     
  Branches     1808     1311     -497     
==========================================
- Hits        17690    17645      -45     
- Misses       3559     3651      +92     
+ Partials       86       24      -62     
Files with missing lines Coverage Δ
src/commands/mirror_node.ts 70.94% <ø> (-0.08%) ⬇️
src/commands/network.ts 65.82% <ø> (-0.04%) ⬇️
src/commands/node/handlers.ts 85.55% <100.00%> (-0.01%) ⬇️
src/commands/relay.ts 81.95% <ø> (-0.08%) ⬇️
src/core/config/remote/remote_config_manager.ts 67.95% <27.27%> (-9.26%) ⬇️

... and 23 files with indirect coverage changes

Impacted file tree graph

@jeromy-cannon jeromy-cannon added PR: Ready to Merge A pull request that is ready to merge. and removed PR: Needs Approval A pull request that needs reviews and approvals. labels Jan 17, 2025
@jeromy-cannon jeromy-cannon merged commit 473a650 into main Jan 18, 2025
42 of 44 checks passed
@jeromy-cannon jeromy-cannon deleted the 01171-update-solo-to-load-remote-config-near-entry-point branch January 18, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Ready to Merge A pull request that is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update Solo to load remote config near entry point
2 participants