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

core/aggsigdb: integrate into simnet #270

Merged
merged 3 commits into from
Mar 29, 2022
Merged

Conversation

leolara
Copy link
Contributor

@leolara leolara commented Mar 23, 2022

Integrate AggSigDB memory implementation into dependency injection and lifecycle launching

category: feature
ticket: #220

@leolara leolara requested a review from corverroos March 23, 2022 15:44
@corverroos corverroos changed the title core/aggsigdb: integrate into dependency injection the new Memory based aggSigDB core/aggsigdb: integrate into simnet Mar 23, 2022
Copy link
Contributor

@corverroos corverroos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment, otherwise looks good!

app/app.go Outdated
@@ -315,6 +315,7 @@ func wireSimNetCoreWorkflow(life *lifecycle.Manager, conf Config, manifest Manif

life.RegisterStart(lifecycle.AsyncAppCtx, lifecycle.StartLeaderCast, lifecycle.HookFunc(consensus.Run))
life.RegisterStart(lifecycle.AsyncBackground, lifecycle.StartScheduler, lifecycle.HookFuncErr(sched.Run))
life.RegisterStart(lifecycle.AsyncAppCtx, lifecycle.StartScheduler, lifecycle.HookFuncCtx(aggSigDB.Run))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to add a lifecycle.AggSigDB enum that is ordered correctly, can start this first, so before StartMonitoringAPI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean I need to add a lifecycle enum before StartMonitoringAPI, so this is not the first one anymore? And then add the use of that enum to the lifecycle manager?

Wouldn't be better to have a dependecy graph of services started by the lifecycle manager, I have implemented that before

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is func (i OrderStart) String() so optimized?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, ok, it is autogenerated, I see

@leolara
Copy link
Contributor Author

leolara commented Mar 28, 2022

@corverroos Please, check, I think I did what you wanted

@leolara leolara merged commit 6d9ae0a into main Mar 29, 2022
@leolara leolara deleted the leo/integrate-aggsigdb-memdb branch March 29, 2022 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants