[node-split] Improve separation between server and client mode #6521
+445
−392
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jellonek
requested review from
dshulyak,
fasmat,
poszu,
ivan4th and
acud
as code owners
This was referenced Dec 5, 2024
... which are required by systest
poszu
approved these changes
Dec 13, 2024
| @@ -390,7 +390,8 @@ func (mgr *PostSetupManager) commitmentAtx(ctx context.Context, dataDir string, | |||
| } | |||
|
|
|||
| // if this node has not published an ATX select the best ATX with `findCommitmentAtx` | |||
| return mgr.findCommitmentAtx(ctx) | |||
| // TODO: replace with highestATX api call | |||
There was a problem hiding this comment.
Just a note for later: the node-service already has an endpoint to query a positioning ATX. This can be used for the commitment ATX (it would be the same ATX).
| @@ -1526,20 +1560,30 @@ func (app *App) listenToUpdates(ctx context.Context) { | |||
| } | |||
|
|
|||
| func (app *App) startServices(ctx context.Context) error { | |||
There was a problem hiding this comment.
How about having:
func (app *App) startNodeServices(ctx context.Context) error {} // for node-service
func (app *App) startSmeshingServices(ctx context.Context) error {} // for smesher-serviceThen don't check against nil inside. The code would crash if a service expected to be started in a given configuration weren't configured instead of silently not being started at all.
There was a problem hiding this comment.
That's the plan for future non-PoC version. Moreover the plan is to have separate entry points/binaries for bootstrap, normal mode, server mode, client mode.
jellonek
force-pushed
the
nodesplit/tuneseparation
branch
5 times, most recently
from
660b15c to
c758264
Compare
tryBuild failed: |
jellonek
force-pushed
the
nodesplit/tuneseparation
branch
from
c758264 to
c1615a0
Compare
jellonek
force-pushed
the
nodesplit/tuneseparation
branch
from
c7fb9b3 to
0b54bcd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Extracted from #6506 and repased on top of current node-split-poc branch.